All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
@ 2013-11-17  1:20 ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17  1:20 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Marek Vasut, Shawn Guo, Richard Zhu, Tejun Heo, Linux-IDE

The same code for enabling and disabling SATA clock was found in multiple
places in the driver. Implement functions that enable/disable the SATA clock
and use them in such places instead of duplicating the code.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 drivers/ata/ahci_imx.c | 133 ++++++++++++++++++++++++++++---------------------
 1 file changed, 75 insertions(+), 58 deletions(-)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index ae2d73f..c7ee505 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
 module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
 MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support, 1=support)");
 
+static int imx_sata_clock_enable(struct device *dev, bool config)
+{
+	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
+	int ret;
+
+	imxpriv->gpr =
+		syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (IS_ERR(imxpriv->gpr)) {
+		dev_err(dev, "failed to find fsl,imx6q-iomux-gpr regmap\n");
+		return PTR_ERR(imxpriv->gpr);
+	}
+
+	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
+	if (ret < 0) {
+		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * set PHY Paremeters, two steps to configure the GPR13,
+	 * one write for rest of parameters, mask of first write
+	 * is 0x07fffffd, and the other one write for setting
+	 * the mpll_clk_en.
+	 */
+	if (config) {
+		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
+				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
+				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
+				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
+				| IMX6Q_GPR13_SATA_MPLL_SS_EN
+				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
+				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
+				| IMX6Q_GPR13_SATA_TX_LVL_MASK
+				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
+				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
+				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
+				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
+				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
+				| IMX6Q_GPR13_SATA_MPLL_SS_EN
+				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
+				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
+				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
+	}
+	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
+			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
+
+	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
+	if (ret < 0) {
+		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void imx_sata_clock_disable(struct device *dev)
+{
+	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
+
+	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+			   IMX6Q_GPR13_SATA_MPLL_CLK_EN,
+			   !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
+	clk_disable_unprepare(imxpriv->sata_ref_clk);
+}
+
 static void ahci_imx_error_handler(struct ata_port *ap)
 {
 	u32 reg_val;
@@ -72,10 +139,7 @@ static void ahci_imx_error_handler(struct ata_port *ap)
 	 */
 	reg_val = readl(mmio + PORT_PHY_CTL);
 	writel(reg_val | PORT_PHY_CTL_PDDQ_LOC, mmio + PORT_PHY_CTL);
-	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-			!IMX6Q_GPR13_SATA_MPLL_CLK_EN);
-	clk_disable_unprepare(imxpriv->sata_ref_clk);
+	imx_sata_clock_disable(ap->dev);
 	imxpriv->no_device = true;
 }
 
@@ -97,44 +161,10 @@ static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
 	unsigned int reg_val;
 	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
 
-	imxpriv->gpr =
-		syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
-	if (IS_ERR(imxpriv->gpr)) {
-		dev_err(dev, "failed to find fsl,imx6q-iomux-gpr regmap\n");
-		return PTR_ERR(imxpriv->gpr);
-	}
-
-	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
-	if (ret < 0) {
-		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
+	ret = imx_sata_clock_enable(dev, true);
+	if (ret < 0)
 		return ret;
-	}
 
-	/*
-	 * set PHY Paremeters, two steps to configure the GPR13,
-	 * one write for rest of parameters, mask of first write
-	 * is 0x07fffffd, and the other one write for setting
-	 * the mpll_clk_en.
-	 */
-	regmap_update_bits(imxpriv->gpr, 0x34, IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
-			| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
-			| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
-			| IMX6Q_GPR13_SATA_SPD_MODE_MASK
-			| IMX6Q_GPR13_SATA_MPLL_SS_EN
-			| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
-			| IMX6Q_GPR13_SATA_TX_BOOST_MASK
-			| IMX6Q_GPR13_SATA_TX_LVL_MASK
-			| IMX6Q_GPR13_SATA_TX_EDGE_RATE
-			, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
-			| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
-			| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
-			| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
-			| IMX6Q_GPR13_SATA_MPLL_SS_EN
-			| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
-			| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
-			| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
-	regmap_update_bits(imxpriv->gpr, 0x34, IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
 	usleep_range(100, 200);
 
 	/*
@@ -163,11 +193,7 @@ static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
 
 static void imx6q_sata_exit(struct device *dev)
 {
-	struct imx_ahci_priv *imxpriv =  dev_get_drvdata(dev->parent);
-
-	regmap_update_bits(imxpriv->gpr, 0x34, IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-			!IMX6Q_GPR13_SATA_MPLL_CLK_EN);
-	clk_disable_unprepare(imxpriv->sata_ref_clk);
+	imx_sata_clock_disable(dev);
 }
 
 static int imx_ahci_suspend(struct device *dev)
@@ -178,12 +204,8 @@ static int imx_ahci_suspend(struct device *dev)
 	 * If no_device is set, The CLKs had been gated off in the
 	 * initialization so don't do it again here.
 	 */
-	if (!imxpriv->no_device) {
-		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-				IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-				!IMX6Q_GPR13_SATA_MPLL_CLK_EN);
-		clk_disable_unprepare(imxpriv->sata_ref_clk);
-	}
+	if (!imxpriv->no_device)
+		imx_sata_clock_disable(dev);
 
 	return 0;
 }
@@ -194,15 +216,10 @@ static int imx_ahci_resume(struct device *dev)
 	int ret;
 
 	if (!imxpriv->no_device) {
-		ret = clk_prepare_enable(imxpriv->sata_ref_clk);
-		if (ret < 0) {
-			dev_err(dev, "pre-enable sata_ref clock err:%d\n", ret);
+		ret = imx_sata_clock_enable(dev, false);
+		if (ret < 0)
 			return ret;
-		}
 
-		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-				IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-				IMX6Q_GPR13_SATA_MPLL_CLK_EN);
 		usleep_range(1000, 2000);
 	}
 
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
@ 2013-11-17  1:20 ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17  1:20 UTC (permalink / raw)
  To: linux-arm-kernel

The same code for enabling and disabling SATA clock was found in multiple
places in the driver. Implement functions that enable/disable the SATA clock
and use them in such places instead of duplicating the code.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 drivers/ata/ahci_imx.c | 133 ++++++++++++++++++++++++++++---------------------
 1 file changed, 75 insertions(+), 58 deletions(-)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index ae2d73f..c7ee505 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
 module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
 MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support, 1=support)");
 
+static int imx_sata_clock_enable(struct device *dev, bool config)
+{
+	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
+	int ret;
+
+	imxpriv->gpr =
+		syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (IS_ERR(imxpriv->gpr)) {
+		dev_err(dev, "failed to find fsl,imx6q-iomux-gpr regmap\n");
+		return PTR_ERR(imxpriv->gpr);
+	}
+
+	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
+	if (ret < 0) {
+		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * set PHY Paremeters, two steps to configure the GPR13,
+	 * one write for rest of parameters, mask of first write
+	 * is 0x07fffffd, and the other one write for setting
+	 * the mpll_clk_en.
+	 */
+	if (config) {
+		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
+				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
+				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
+				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
+				| IMX6Q_GPR13_SATA_MPLL_SS_EN
+				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
+				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
+				| IMX6Q_GPR13_SATA_TX_LVL_MASK
+				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
+				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
+				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
+				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
+				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
+				| IMX6Q_GPR13_SATA_MPLL_SS_EN
+				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
+				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
+				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
+	}
+	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
+			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
+
+	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
+	if (ret < 0) {
+		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void imx_sata_clock_disable(struct device *dev)
+{
+	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
+
+	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+			   IMX6Q_GPR13_SATA_MPLL_CLK_EN,
+			   !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
+	clk_disable_unprepare(imxpriv->sata_ref_clk);
+}
+
 static void ahci_imx_error_handler(struct ata_port *ap)
 {
 	u32 reg_val;
@@ -72,10 +139,7 @@ static void ahci_imx_error_handler(struct ata_port *ap)
 	 */
 	reg_val = readl(mmio + PORT_PHY_CTL);
 	writel(reg_val | PORT_PHY_CTL_PDDQ_LOC, mmio + PORT_PHY_CTL);
-	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-			!IMX6Q_GPR13_SATA_MPLL_CLK_EN);
-	clk_disable_unprepare(imxpriv->sata_ref_clk);
+	imx_sata_clock_disable(ap->dev);
 	imxpriv->no_device = true;
 }
 
@@ -97,44 +161,10 @@ static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
 	unsigned int reg_val;
 	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
 
-	imxpriv->gpr =
-		syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
-	if (IS_ERR(imxpriv->gpr)) {
-		dev_err(dev, "failed to find fsl,imx6q-iomux-gpr regmap\n");
-		return PTR_ERR(imxpriv->gpr);
-	}
-
-	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
-	if (ret < 0) {
-		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
+	ret = imx_sata_clock_enable(dev, true);
+	if (ret < 0)
 		return ret;
-	}
 
-	/*
-	 * set PHY Paremeters, two steps to configure the GPR13,
-	 * one write for rest of parameters, mask of first write
-	 * is 0x07fffffd, and the other one write for setting
-	 * the mpll_clk_en.
-	 */
-	regmap_update_bits(imxpriv->gpr, 0x34, IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
-			| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
-			| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
-			| IMX6Q_GPR13_SATA_SPD_MODE_MASK
-			| IMX6Q_GPR13_SATA_MPLL_SS_EN
-			| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
-			| IMX6Q_GPR13_SATA_TX_BOOST_MASK
-			| IMX6Q_GPR13_SATA_TX_LVL_MASK
-			| IMX6Q_GPR13_SATA_TX_EDGE_RATE
-			, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
-			| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
-			| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
-			| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
-			| IMX6Q_GPR13_SATA_MPLL_SS_EN
-			| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
-			| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
-			| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
-	regmap_update_bits(imxpriv->gpr, 0x34, IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
 	usleep_range(100, 200);
 
 	/*
@@ -163,11 +193,7 @@ static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
 
 static void imx6q_sata_exit(struct device *dev)
 {
-	struct imx_ahci_priv *imxpriv =  dev_get_drvdata(dev->parent);
-
-	regmap_update_bits(imxpriv->gpr, 0x34, IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-			!IMX6Q_GPR13_SATA_MPLL_CLK_EN);
-	clk_disable_unprepare(imxpriv->sata_ref_clk);
+	imx_sata_clock_disable(dev);
 }
 
 static int imx_ahci_suspend(struct device *dev)
@@ -178,12 +204,8 @@ static int imx_ahci_suspend(struct device *dev)
 	 * If no_device is set, The CLKs had been gated off in the
 	 * initialization so don't do it again here.
 	 */
-	if (!imxpriv->no_device) {
-		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-				IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-				!IMX6Q_GPR13_SATA_MPLL_CLK_EN);
-		clk_disable_unprepare(imxpriv->sata_ref_clk);
-	}
+	if (!imxpriv->no_device)
+		imx_sata_clock_disable(dev);
 
 	return 0;
 }
@@ -194,15 +216,10 @@ static int imx_ahci_resume(struct device *dev)
 	int ret;
 
 	if (!imxpriv->no_device) {
-		ret = clk_prepare_enable(imxpriv->sata_ref_clk);
-		if (ret < 0) {
-			dev_err(dev, "pre-enable sata_ref clock err:%d\n", ret);
+		ret = imx_sata_clock_enable(dev, false);
+		if (ret < 0)
 			return ret;
-		}
 
-		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-				IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-				IMX6Q_GPR13_SATA_MPLL_CLK_EN);
 		usleep_range(1000, 2000);
 	}
 
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 2/5] ahci: imx: Add i.MX53 support
  2013-11-17  1:20 ` Marek Vasut
@ 2013-11-17  1:20   ` Marek Vasut
  -1 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17  1:20 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Marek Vasut, Shawn Guo, Richard Zhu, Tejun Heo, Linux-IDE

Add minor adjustments to support i.MX53 SATA port as well as i.MX6Q one.
The difference here is mostly the clock which need to be enabled and also
the lack of need of programming IOMUXC registers on i.MX53. All of which
is well handles in the clock enable/disable functions. Note that this patch
also cleans up the names of the common functions, so they don't read imx6q_*
but imx_* instead.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 drivers/ata/ahci_imx.c | 198 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 128 insertions(+), 70 deletions(-)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index c7ee505..e6854593 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -34,10 +34,23 @@ enum {
 	HOST_TIMER1MS = 0xe0,			/* Timer 1-ms */
 };
 
+enum ahci_imx_type {
+	AHCI_IMX53,
+	AHCI_IMX6Q,
+};
+
 struct imx_ahci_priv {
 	struct platform_device *ahci_pdev;
+	enum ahci_imx_type type;
+
+	/* i.MX53 clock */
+	struct clk *sata_gate_clk;
+	struct clk *sata_phy_clk;
+	/* i.MX6Q clock */
 	struct clk *sata_ref_clk;
+	/* Common clock */
 	struct clk *ahb_clk;
+
 	struct regmap *gpr;
 	bool no_device;
 	bool first_time;
@@ -52,53 +65,73 @@ static int imx_sata_clock_enable(struct device *dev, bool config)
 	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
 	int ret;
 
-	imxpriv->gpr =
-		syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
-	if (IS_ERR(imxpriv->gpr)) {
-		dev_err(dev, "failed to find fsl,imx6q-iomux-gpr regmap\n");
-		return PTR_ERR(imxpriv->gpr);
-	}
-
-	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
-	if (ret < 0) {
-		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
-		return ret;
-	}
+	if (imxpriv->type == AHCI_IMX53) {
+		ret = clk_prepare_enable(imxpriv->sata_gate_clk);
+		if (ret < 0) {
+			dev_err(dev, "prepare-enable sata_gate clock err:%d\n",
+				ret);
+			return ret;
+		}
 
-	/*
-	 * set PHY Paremeters, two steps to configure the GPR13,
-	 * one write for rest of parameters, mask of first write
-	 * is 0x07fffffd, and the other one write for setting
-	 * the mpll_clk_en.
-	 */
-	if (config) {
+		ret = clk_prepare_enable(imxpriv->sata_phy_clk);
+		if (ret < 0) {
+			clk_disable_unprepare(imxpriv->sata_gate_clk);
+			dev_err(dev, "prepare-enable sata_phy clock err:%d\n",
+				ret);
+			return ret;
+		}
+	} else if (imxpriv->type == AHCI_IMX6Q) {
+		imxpriv->gpr = syscon_regmap_lookup_by_compatible(
+							"fsl,imx6q-iomuxc-gpr");
+		if (IS_ERR(imxpriv->gpr)) {
+			dev_err(dev,
+				"failed to find fsl,imx6q-iomux-gpr regmap\n");
+			return PTR_ERR(imxpriv->gpr);
+		}
+
+		ret = clk_prepare_enable(imxpriv->sata_ref_clk);
+		if (ret < 0) {
+			dev_err(dev, "prepare-enable sata_ref clock err:%d\n",
+				ret);
+			return ret;
+		}
+
+		/*
+		 * set PHY Paremeters, two steps to configure the GPR13,
+		 * one write for rest of parameters, mask of first write
+		 * is 0x07fffffd, and the other one write for setting
+		 * the mpll_clk_en.
+		 */
+		if (config) {
+			regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+					IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
+					| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
+					| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
+					| IMX6Q_GPR13_SATA_SPD_MODE_MASK
+					| IMX6Q_GPR13_SATA_MPLL_SS_EN
+					| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
+					| IMX6Q_GPR13_SATA_TX_BOOST_MASK
+					| IMX6Q_GPR13_SATA_TX_LVL_MASK
+					| IMX6Q_GPR13_SATA_TX_EDGE_RATE
+					, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
+					| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
+					| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
+					| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
+					| IMX6Q_GPR13_SATA_MPLL_SS_EN
+					| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
+					| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
+					| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
+		}
 		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
-				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
-				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
-				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
-				| IMX6Q_GPR13_SATA_MPLL_SS_EN
-				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
-				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
-				| IMX6Q_GPR13_SATA_TX_LVL_MASK
-				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
-				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
-				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
-				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
-				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
-				| IMX6Q_GPR13_SATA_MPLL_SS_EN
-				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
-				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
-				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
-	}
-	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
+				IMX6Q_GPR13_SATA_MPLL_CLK_EN,
+				IMX6Q_GPR13_SATA_MPLL_CLK_EN);
 
-	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
-	if (ret < 0) {
-		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
-		return ret;
+		ret = clk_prepare_enable(imxpriv->sata_ref_clk);
+		if (ret < 0) {
+			dev_err(dev, "prepare-enable sata_ref clock err:%d\n",
+				ret);
+			return ret;
+		}
 	}
 
 	return 0;
@@ -108,10 +141,15 @@ static void imx_sata_clock_disable(struct device *dev)
 {
 	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
 
-	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-			   IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-			   !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
-	clk_disable_unprepare(imxpriv->sata_ref_clk);
+	if (imxpriv->type == AHCI_IMX53) {
+		clk_disable_unprepare(imxpriv->sata_phy_clk);
+		clk_disable_unprepare(imxpriv->sata_gate_clk);
+	} else if (imxpriv->type == AHCI_IMX6Q) {
+		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+				   IMX6Q_GPR13_SATA_MPLL_CLK_EN,
+				   !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
+		clk_disable_unprepare(imxpriv->sata_ref_clk);
+	}
 }
 
 static void ahci_imx_error_handler(struct ata_port *ap)
@@ -155,7 +193,7 @@ static const struct ata_port_info ahci_imx_port_info = {
 	.port_ops	= &ahci_imx_ops,
 };
 
-static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
+static int imx_sata_init(struct device *dev, void __iomem *mmio)
 {
 	int ret = 0;
 	unsigned int reg_val;
@@ -191,7 +229,7 @@ static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
 	return 0;
 }
 
-static void imx6q_sata_exit(struct device *dev)
+static void imx_sata_exit(struct device *dev)
 {
 	imx_sata_clock_disable(dev);
 }
@@ -226,16 +264,18 @@ static int imx_ahci_resume(struct device *dev)
 	return 0;
 }
 
-static struct ahci_platform_data imx6q_sata_pdata = {
-	.init = imx6q_sata_init,
-	.exit = imx6q_sata_exit,
-	.ata_port_info = &ahci_imx_port_info,
-	.suspend = imx_ahci_suspend,
-	.resume = imx_ahci_resume,
+static struct ahci_platform_data imx_sata_pdata = {
+	.init		= imx_sata_init,
+	.exit		= imx_sata_exit,
+	.ata_port_info	= &ahci_imx_port_info,
+	.suspend	= imx_ahci_suspend,
+	.resume		= imx_ahci_resume,
+
 };
 
 static const struct of_device_id imx_ahci_of_match[] = {
-	{ .compatible = "fsl,imx6q-ahci", .data = &imx6q_sata_pdata},
+	{ .compatible = "fsl,imx53-ahci", .data = (void *)AHCI_IMX53 },
+	{ .compatible = "fsl,imx6q-ahci", .data = (void *)AHCI_IMX6Q },
 	{},
 };
 MODULE_DEVICE_TABLE(of, imx_ahci_of_match);
@@ -245,12 +285,20 @@ static int imx_ahci_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource *mem, *irq, res[2];
 	const struct of_device_id *of_id;
+	enum ahci_imx_type type;
 	const struct ahci_platform_data *pdata = NULL;
 	struct imx_ahci_priv *imxpriv;
 	struct device *ahci_dev;
 	struct platform_device *ahci_pdev;
 	int ret;
 
+	of_id = of_match_device(imx_ahci_of_match, dev);
+	if (!of_id)
+		return -EINVAL;
+
+	type = (enum ahci_imx_type)of_id->data;
+	pdata = &imx_sata_pdata;
+
 	imxpriv = devm_kzalloc(dev, sizeof(*imxpriv), GFP_KERNEL);
 	if (!imxpriv) {
 		dev_err(dev, "can't alloc ahci_host_priv\n");
@@ -266,6 +314,8 @@ static int imx_ahci_probe(struct platform_device *pdev)
 
 	imxpriv->no_device = false;
 	imxpriv->first_time = true;
+	imxpriv->type = type;
+
 	imxpriv->ahb_clk = devm_clk_get(dev, "ahb");
 	if (IS_ERR(imxpriv->ahb_clk)) {
 		dev_err(dev, "can't get ahb clock.\n");
@@ -273,24 +323,32 @@ static int imx_ahci_probe(struct platform_device *pdev)
 		goto err_out;
 	}
 
-	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");
-		ret = PTR_ERR(imxpriv->sata_ref_clk);
-		goto err_out;
+	if (type == AHCI_IMX53) {
+		imxpriv->sata_gate_clk = devm_clk_get(dev, "sata_gate");
+		if (IS_ERR(imxpriv->sata_gate_clk)) {
+			dev_err(dev, "can't get sata_gate clock.\n");
+			ret = PTR_ERR(imxpriv->sata_gate_clk);
+			goto err_out;
+		}
+
+		imxpriv->sata_phy_clk = devm_clk_get(dev, "sata_phy");
+		if (IS_ERR(imxpriv->sata_phy_clk)) {
+			dev_err(dev, "can't get sata_phy clock.\n");
+			ret = PTR_ERR(imxpriv->sata_phy_clk);
+			goto err_out;
+		}
+	} else if (type == AHCI_IMX6Q) {
+		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");
+			ret = PTR_ERR(imxpriv->sata_ref_clk);
+			goto err_out;
+		}
 	}
 
 	imxpriv->ahci_pdev = ahci_pdev;
 	platform_set_drvdata(pdev, imxpriv);
 
-	of_id = of_match_device(imx_ahci_of_match, dev);
-	if (of_id) {
-		pdata = of_id->data;
-	} else {
-		ret = -EINVAL;
-		goto err_out;
-	}
-
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (!mem || !irq) {
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 2/5] ahci: imx: Add i.MX53 support
@ 2013-11-17  1:20   ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17  1:20 UTC (permalink / raw)
  To: linux-arm-kernel

Add minor adjustments to support i.MX53 SATA port as well as i.MX6Q one.
The difference here is mostly the clock which need to be enabled and also
the lack of need of programming IOMUXC registers on i.MX53. All of which
is well handles in the clock enable/disable functions. Note that this patch
also cleans up the names of the common functions, so they don't read imx6q_*
but imx_* instead.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 drivers/ata/ahci_imx.c | 198 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 128 insertions(+), 70 deletions(-)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index c7ee505..e6854593 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -34,10 +34,23 @@ enum {
 	HOST_TIMER1MS = 0xe0,			/* Timer 1-ms */
 };
 
+enum ahci_imx_type {
+	AHCI_IMX53,
+	AHCI_IMX6Q,
+};
+
 struct imx_ahci_priv {
 	struct platform_device *ahci_pdev;
+	enum ahci_imx_type type;
+
+	/* i.MX53 clock */
+	struct clk *sata_gate_clk;
+	struct clk *sata_phy_clk;
+	/* i.MX6Q clock */
 	struct clk *sata_ref_clk;
+	/* Common clock */
 	struct clk *ahb_clk;
+
 	struct regmap *gpr;
 	bool no_device;
 	bool first_time;
@@ -52,53 +65,73 @@ static int imx_sata_clock_enable(struct device *dev, bool config)
 	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
 	int ret;
 
-	imxpriv->gpr =
-		syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
-	if (IS_ERR(imxpriv->gpr)) {
-		dev_err(dev, "failed to find fsl,imx6q-iomux-gpr regmap\n");
-		return PTR_ERR(imxpriv->gpr);
-	}
-
-	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
-	if (ret < 0) {
-		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
-		return ret;
-	}
+	if (imxpriv->type == AHCI_IMX53) {
+		ret = clk_prepare_enable(imxpriv->sata_gate_clk);
+		if (ret < 0) {
+			dev_err(dev, "prepare-enable sata_gate clock err:%d\n",
+				ret);
+			return ret;
+		}
 
-	/*
-	 * set PHY Paremeters, two steps to configure the GPR13,
-	 * one write for rest of parameters, mask of first write
-	 * is 0x07fffffd, and the other one write for setting
-	 * the mpll_clk_en.
-	 */
-	if (config) {
+		ret = clk_prepare_enable(imxpriv->sata_phy_clk);
+		if (ret < 0) {
+			clk_disable_unprepare(imxpriv->sata_gate_clk);
+			dev_err(dev, "prepare-enable sata_phy clock err:%d\n",
+				ret);
+			return ret;
+		}
+	} else if (imxpriv->type == AHCI_IMX6Q) {
+		imxpriv->gpr = syscon_regmap_lookup_by_compatible(
+							"fsl,imx6q-iomuxc-gpr");
+		if (IS_ERR(imxpriv->gpr)) {
+			dev_err(dev,
+				"failed to find fsl,imx6q-iomux-gpr regmap\n");
+			return PTR_ERR(imxpriv->gpr);
+		}
+
+		ret = clk_prepare_enable(imxpriv->sata_ref_clk);
+		if (ret < 0) {
+			dev_err(dev, "prepare-enable sata_ref clock err:%d\n",
+				ret);
+			return ret;
+		}
+
+		/*
+		 * set PHY Paremeters, two steps to configure the GPR13,
+		 * one write for rest of parameters, mask of first write
+		 * is 0x07fffffd, and the other one write for setting
+		 * the mpll_clk_en.
+		 */
+		if (config) {
+			regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+					IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
+					| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
+					| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
+					| IMX6Q_GPR13_SATA_SPD_MODE_MASK
+					| IMX6Q_GPR13_SATA_MPLL_SS_EN
+					| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
+					| IMX6Q_GPR13_SATA_TX_BOOST_MASK
+					| IMX6Q_GPR13_SATA_TX_LVL_MASK
+					| IMX6Q_GPR13_SATA_TX_EDGE_RATE
+					, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
+					| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
+					| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
+					| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
+					| IMX6Q_GPR13_SATA_MPLL_SS_EN
+					| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
+					| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
+					| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
+		}
 		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
-				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
-				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
-				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
-				| IMX6Q_GPR13_SATA_MPLL_SS_EN
-				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
-				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
-				| IMX6Q_GPR13_SATA_TX_LVL_MASK
-				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
-				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
-				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
-				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
-				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
-				| IMX6Q_GPR13_SATA_MPLL_SS_EN
-				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
-				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
-				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
-	}
-	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
+				IMX6Q_GPR13_SATA_MPLL_CLK_EN,
+				IMX6Q_GPR13_SATA_MPLL_CLK_EN);
 
-	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
-	if (ret < 0) {
-		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
-		return ret;
+		ret = clk_prepare_enable(imxpriv->sata_ref_clk);
+		if (ret < 0) {
+			dev_err(dev, "prepare-enable sata_ref clock err:%d\n",
+				ret);
+			return ret;
+		}
 	}
 
 	return 0;
@@ -108,10 +141,15 @@ static void imx_sata_clock_disable(struct device *dev)
 {
 	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
 
-	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
-			   IMX6Q_GPR13_SATA_MPLL_CLK_EN,
-			   !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
-	clk_disable_unprepare(imxpriv->sata_ref_clk);
+	if (imxpriv->type == AHCI_IMX53) {
+		clk_disable_unprepare(imxpriv->sata_phy_clk);
+		clk_disable_unprepare(imxpriv->sata_gate_clk);
+	} else if (imxpriv->type == AHCI_IMX6Q) {
+		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+				   IMX6Q_GPR13_SATA_MPLL_CLK_EN,
+				   !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
+		clk_disable_unprepare(imxpriv->sata_ref_clk);
+	}
 }
 
 static void ahci_imx_error_handler(struct ata_port *ap)
@@ -155,7 +193,7 @@ static const struct ata_port_info ahci_imx_port_info = {
 	.port_ops	= &ahci_imx_ops,
 };
 
-static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
+static int imx_sata_init(struct device *dev, void __iomem *mmio)
 {
 	int ret = 0;
 	unsigned int reg_val;
@@ -191,7 +229,7 @@ static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
 	return 0;
 }
 
-static void imx6q_sata_exit(struct device *dev)
+static void imx_sata_exit(struct device *dev)
 {
 	imx_sata_clock_disable(dev);
 }
@@ -226,16 +264,18 @@ static int imx_ahci_resume(struct device *dev)
 	return 0;
 }
 
-static struct ahci_platform_data imx6q_sata_pdata = {
-	.init = imx6q_sata_init,
-	.exit = imx6q_sata_exit,
-	.ata_port_info = &ahci_imx_port_info,
-	.suspend = imx_ahci_suspend,
-	.resume = imx_ahci_resume,
+static struct ahci_platform_data imx_sata_pdata = {
+	.init		= imx_sata_init,
+	.exit		= imx_sata_exit,
+	.ata_port_info	= &ahci_imx_port_info,
+	.suspend	= imx_ahci_suspend,
+	.resume		= imx_ahci_resume,
+
 };
 
 static const struct of_device_id imx_ahci_of_match[] = {
-	{ .compatible = "fsl,imx6q-ahci", .data = &imx6q_sata_pdata},
+	{ .compatible = "fsl,imx53-ahci", .data = (void *)AHCI_IMX53 },
+	{ .compatible = "fsl,imx6q-ahci", .data = (void *)AHCI_IMX6Q },
 	{},
 };
 MODULE_DEVICE_TABLE(of, imx_ahci_of_match);
@@ -245,12 +285,20 @@ static int imx_ahci_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource *mem, *irq, res[2];
 	const struct of_device_id *of_id;
+	enum ahci_imx_type type;
 	const struct ahci_platform_data *pdata = NULL;
 	struct imx_ahci_priv *imxpriv;
 	struct device *ahci_dev;
 	struct platform_device *ahci_pdev;
 	int ret;
 
+	of_id = of_match_device(imx_ahci_of_match, dev);
+	if (!of_id)
+		return -EINVAL;
+
+	type = (enum ahci_imx_type)of_id->data;
+	pdata = &imx_sata_pdata;
+
 	imxpriv = devm_kzalloc(dev, sizeof(*imxpriv), GFP_KERNEL);
 	if (!imxpriv) {
 		dev_err(dev, "can't alloc ahci_host_priv\n");
@@ -266,6 +314,8 @@ static int imx_ahci_probe(struct platform_device *pdev)
 
 	imxpriv->no_device = false;
 	imxpriv->first_time = true;
+	imxpriv->type = type;
+
 	imxpriv->ahb_clk = devm_clk_get(dev, "ahb");
 	if (IS_ERR(imxpriv->ahb_clk)) {
 		dev_err(dev, "can't get ahb clock.\n");
@@ -273,24 +323,32 @@ static int imx_ahci_probe(struct platform_device *pdev)
 		goto err_out;
 	}
 
-	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");
-		ret = PTR_ERR(imxpriv->sata_ref_clk);
-		goto err_out;
+	if (type == AHCI_IMX53) {
+		imxpriv->sata_gate_clk = devm_clk_get(dev, "sata_gate");
+		if (IS_ERR(imxpriv->sata_gate_clk)) {
+			dev_err(dev, "can't get sata_gate clock.\n");
+			ret = PTR_ERR(imxpriv->sata_gate_clk);
+			goto err_out;
+		}
+
+		imxpriv->sata_phy_clk = devm_clk_get(dev, "sata_phy");
+		if (IS_ERR(imxpriv->sata_phy_clk)) {
+			dev_err(dev, "can't get sata_phy clock.\n");
+			ret = PTR_ERR(imxpriv->sata_phy_clk);
+			goto err_out;
+		}
+	} else if (type == AHCI_IMX6Q) {
+		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");
+			ret = PTR_ERR(imxpriv->sata_ref_clk);
+			goto err_out;
+		}
 	}
 
 	imxpriv->ahci_pdev = ahci_pdev;
 	platform_set_drvdata(pdev, imxpriv);
 
-	of_id = of_match_device(imx_ahci_of_match, dev);
-	if (of_id) {
-		pdata = of_id->data;
-	} else {
-		ret = -EINVAL;
-		goto err_out;
-	}
-
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (!mem || !irq) {
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 3/5] ARM: imx: imx53: Add SATA PHY clock
  2013-11-17  1:20 ` Marek Vasut
@ 2013-11-17  1:20   ` Marek Vasut
  -1 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17  1:20 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Marek Vasut, Shawn Guo, Richard Zhu, Tejun Heo, Linux-IDE

Add SATA PHY clock which are derived from the USB PHY1 clock. Note that this
patch derives the SATA PHY clock from USB PHY1 clock gate so that the SATA
driver can ungate both the SATA PHY clock and USB PHY1 clock for the SATA to
work correctly.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 Documentation/devicetree/bindings/clock/imx5-clock.txt | 1 +
 arch/arm/mach-imx/clk-imx51-imx53.c                    | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/imx5-clock.txt b/Documentation/devicetree/bindings/clock/imx5-clock.txt
index 3716b36..8904731 100644
--- a/Documentation/devicetree/bindings/clock/imx5-clock.txt
+++ b/Documentation/devicetree/bindings/clock/imx5-clock.txt
@@ -199,6 +199,7 @@ clocks and IDs.
 	spdif_ipg_gate		185
 	ocram			186
 	sahara_ipg_gate		187
+	sata_phy		188
 
 Examples (for mx53):
 
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
index 3d91172..06bfb4c 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -122,7 +122,7 @@ enum imx5_clks {
 	srtc_gate, pata_gate, sata_gate, spdif_xtal_sel, spdif0_sel,
 	spdif1_sel, spdif0_pred, spdif0_podf, spdif1_pred, spdif1_podf,
 	spdif0_com_s, spdif1_com_sel, spdif0_gate, spdif1_gate, spdif_ipg_gate,
-	ocram, sahara_ipg_gate, clk_max
+	ocram, sahara_ipg_gate, sata_phy, clk_max
 };
 
 static struct clk *clk[clk_max];
@@ -588,6 +588,7 @@ static void __init mx53_clocks_init(struct device_node *np)
 	clk[cko2] = imx_clk_gate2("cko2", "cko2_podf", MXC_CCM_CCOSR, 24);
 	clk[spdif_xtal_sel] = imx_clk_mux("spdif_xtal_sel", MXC_CCM_CSCMR1, 2, 2,
 				mx53_spdif_xtal_sel, ARRAY_SIZE(mx53_spdif_xtal_sel));
+	clk[sata_phy] = imx_clk_fixed_factor("sata_phy", "usb_phy1_gate", 1, 1);
 
 	for (i = 0; i < ARRAY_SIZE(clk); i++)
 		if (IS_ERR(clk[i]))
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 3/5] ARM: imx: imx53: Add SATA PHY clock
@ 2013-11-17  1:20   ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17  1:20 UTC (permalink / raw)
  To: linux-arm-kernel

Add SATA PHY clock which are derived from the USB PHY1 clock. Note that this
patch derives the SATA PHY clock from USB PHY1 clock gate so that the SATA
driver can ungate both the SATA PHY clock and USB PHY1 clock for the SATA to
work correctly.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 Documentation/devicetree/bindings/clock/imx5-clock.txt | 1 +
 arch/arm/mach-imx/clk-imx51-imx53.c                    | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/imx5-clock.txt b/Documentation/devicetree/bindings/clock/imx5-clock.txt
index 3716b36..8904731 100644
--- a/Documentation/devicetree/bindings/clock/imx5-clock.txt
+++ b/Documentation/devicetree/bindings/clock/imx5-clock.txt
@@ -199,6 +199,7 @@ clocks and IDs.
 	spdif_ipg_gate		185
 	ocram			186
 	sahara_ipg_gate		187
+	sata_phy		188
 
 Examples (for mx53):
 
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
index 3d91172..06bfb4c 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -122,7 +122,7 @@ enum imx5_clks {
 	srtc_gate, pata_gate, sata_gate, spdif_xtal_sel, spdif0_sel,
 	spdif1_sel, spdif0_pred, spdif0_podf, spdif1_pred, spdif1_podf,
 	spdif0_com_s, spdif1_com_sel, spdif0_gate, spdif1_gate, spdif_ipg_gate,
-	ocram, sahara_ipg_gate, clk_max
+	ocram, sahara_ipg_gate, sata_phy, clk_max
 };
 
 static struct clk *clk[clk_max];
@@ -588,6 +588,7 @@ static void __init mx53_clocks_init(struct device_node *np)
 	clk[cko2] = imx_clk_gate2("cko2", "cko2_podf", MXC_CCM_CCOSR, 24);
 	clk[spdif_xtal_sel] = imx_clk_mux("spdif_xtal_sel", MXC_CCM_CSCMR1, 2, 2,
 				mx53_spdif_xtal_sel, ARRAY_SIZE(mx53_spdif_xtal_sel));
+	clk[sata_phy] = imx_clk_fixed_factor("sata_phy", "usb_phy1_gate", 1, 1);
 
 	for (i = 0; i < ARRAY_SIZE(clk); i++)
 		if (IS_ERR(clk[i]))
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 4/5] ARM: dts: imx53: Add AHCI SATA binding
  2013-11-17  1:20 ` Marek Vasut
@ 2013-11-17  1:20   ` Marek Vasut
  -1 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17  1:20 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Marek Vasut, Shawn Guo, Richard Zhu, Tejun Heo, Linux-IDE

The AHCI-IMX driver now supports i.MX53 as well. Add DT binding.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 arch/arm/boot/dts/imx53.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index cffdda2..0e92af0 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -85,6 +85,15 @@
 		interrupt-parent = <&tzic>;
 		ranges;
 
+		sata: sata@10000000 {
+			compatible = "fsl,imx53-ahci";
+			reg = <0x10000000 0x1000>;
+			interrupts = <28>;
+			clocks =  <&clks 173>, <&clks 188>, <&clks 5>;
+			clock-names = "sata_gate", "sata_phy", "ahb";
+			status = "disabled";
+		};
+
 		ipu: ipu@18000000 {
 			#crtc-cells = <1>;
 			compatible = "fsl,imx53-ipu";
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 4/5] ARM: dts: imx53: Add AHCI SATA binding
@ 2013-11-17  1:20   ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17  1:20 UTC (permalink / raw)
  To: linux-arm-kernel

The AHCI-IMX driver now supports i.MX53 as well. Add DT binding.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 arch/arm/boot/dts/imx53.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index cffdda2..0e92af0 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -85,6 +85,15 @@
 		interrupt-parent = <&tzic>;
 		ranges;
 
+		sata: sata at 10000000 {
+			compatible = "fsl,imx53-ahci";
+			reg = <0x10000000 0x1000>;
+			interrupts = <28>;
+			clocks =  <&clks 173>, <&clks 188>, <&clks 5>;
+			clock-names = "sata_gate", "sata_phy", "ahb";
+			status = "disabled";
+		};
+
 		ipu: ipu at 18000000 {
 			#crtc-cells = <1>;
 			compatible = "fsl,imx53-ipu";
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 5/5] ARM: dts: imx53: Enable AHCI SATA for M53EVK
  2013-11-17  1:20 ` Marek Vasut
@ 2013-11-17  1:20   ` Marek Vasut
  -1 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17  1:20 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Marek Vasut, Shawn Guo, Richard Zhu, Tejun Heo, Linux-IDE

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 arch/arm/boot/dts/imx53-m53evk.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts
index 658d32a..c45a8d9c 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -44,6 +44,10 @@
 				};
 			};
 		};
+
+		sata@10000000 {
+			status = "okay";
+		};
 	};
 
 	backlight {
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 5/5] ARM: dts: imx53: Enable AHCI SATA for M53EVK
@ 2013-11-17  1:20   ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17  1:20 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 arch/arm/boot/dts/imx53-m53evk.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts
index 658d32a..c45a8d9c 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -44,6 +44,10 @@
 				};
 			};
 		};
+
+		sata at 10000000 {
+			status = "okay";
+		};
 	};
 
 	backlight {
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
  2013-11-17  1:20 ` Marek Vasut
@ 2013-11-17  8:15   ` Lothar Waßmann
  -1 siblings, 0 replies; 30+ messages in thread
From: Lothar Waßmann @ 2013-11-17  8:15 UTC (permalink / raw)
  To: Marek Vasut
  Cc: linux-arm-kernel, Richard Zhu, Tejun Heo, Shawn Guo, Linux-IDE

Hi,

Marek Vasut wrote:
> The same code for enabling and disabling SATA clock was found in multiple
> places in the driver. Implement functions that enable/disable the SATA clock
> and use them in such places instead of duplicating the code.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> ---
>  drivers/ata/ahci_imx.c | 133 ++++++++++++++++++++++++++++---------------------
>  1 file changed, 75 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index ae2d73f..c7ee505 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
>  module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
>  MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support, 1=support)");
>  
> +static int imx_sata_clock_enable(struct device *dev, bool config)
> +{
> +	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
> +	int ret;
> +
> +	imxpriv->gpr =
> +		syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
> +	if (IS_ERR(imxpriv->gpr)) {
> +		dev_err(dev, "failed to find fsl,imx6q-iomux-gpr regmap\n");
> +		return PTR_ERR(imxpriv->gpr);
> +	}
> +
> +	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
> +	if (ret < 0) {
> +		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * set PHY Paremeters, two steps to configure the GPR13,
> +	 * one write for rest of parameters, mask of first write
> +	 * is 0x07fffffd, and the other one write for setting
> +	 * the mpll_clk_en.
> +	 */
> +	if (config) {
> +		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
> +				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
> +				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
> +				| IMX6Q_GPR13_SATA_TX_LVL_MASK
> +				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
> +				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
>
I would prefer the comma to be at the end of the line where one usually
looks for it. Also '#define'ing constants for these bitmasks would make
the code more readable.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
@ 2013-11-17  8:15   ` Lothar Waßmann
  0 siblings, 0 replies; 30+ messages in thread
From: Lothar Waßmann @ 2013-11-17  8:15 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

Marek Vasut wrote:
> The same code for enabling and disabling SATA clock was found in multiple
> places in the driver. Implement functions that enable/disable the SATA clock
> and use them in such places instead of duplicating the code.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> ---
>  drivers/ata/ahci_imx.c | 133 ++++++++++++++++++++++++++++---------------------
>  1 file changed, 75 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index ae2d73f..c7ee505 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
>  module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
>  MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support, 1=support)");
>  
> +static int imx_sata_clock_enable(struct device *dev, bool config)
> +{
> +	struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
> +	int ret;
> +
> +	imxpriv->gpr =
> +		syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
> +	if (IS_ERR(imxpriv->gpr)) {
> +		dev_err(dev, "failed to find fsl,imx6q-iomux-gpr regmap\n");
> +		return PTR_ERR(imxpriv->gpr);
> +	}
> +
> +	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
> +	if (ret < 0) {
> +		dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * set PHY Paremeters, two steps to configure the GPR13,
> +	 * one write for rest of parameters, mask of first write
> +	 * is 0x07fffffd, and the other one write for setting
> +	 * the mpll_clk_en.
> +	 */
> +	if (config) {
> +		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
> +				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
> +				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
> +				| IMX6Q_GPR13_SATA_TX_LVL_MASK
> +				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
> +				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
>
I would prefer the comma to be at the end of the line where one usually
looks for it. Also '#define'ing constants for these bitmasks would make
the code more readable.


Lothar Wa?mann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 4/5] ARM: dts: imx53: Add AHCI SATA binding
  2013-11-17  1:20   ` Marek Vasut
@ 2013-11-18  2:41     ` Shawn Guo
  -1 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2013-11-18  2:41 UTC (permalink / raw)
  To: Marek Vasut; +Cc: linux-arm-kernel, Richard Zhu, Tejun Heo, Linux-IDE

On Sun, Nov 17, 2013 at 02:20:50AM +0100, Marek Vasut wrote:
> The AHCI-IMX driver now supports i.MX53 as well. Add DT binding.

I think this patch can be merged independently without the driver
changes in place.  Also, I understand that the term 'binding' is
referred as binding doc, while what you're patching here is device tree
source.

Shawn

> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> ---
>  arch/arm/boot/dts/imx53.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> index cffdda2..0e92af0 100644
> --- a/arch/arm/boot/dts/imx53.dtsi
> +++ b/arch/arm/boot/dts/imx53.dtsi
> @@ -85,6 +85,15 @@
>  		interrupt-parent = <&tzic>;
>  		ranges;
>  
> +		sata: sata@10000000 {
> +			compatible = "fsl,imx53-ahci";
> +			reg = <0x10000000 0x1000>;
> +			interrupts = <28>;
> +			clocks =  <&clks 173>, <&clks 188>, <&clks 5>;
> +			clock-names = "sata_gate", "sata_phy", "ahb";
> +			status = "disabled";
> +		};
> +
>  		ipu: ipu@18000000 {
>  			#crtc-cells = <1>;
>  			compatible = "fsl,imx53-ipu";
> -- 
> 1.8.4.2
> 


^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH 4/5] ARM: dts: imx53: Add AHCI SATA binding
@ 2013-11-18  2:41     ` Shawn Guo
  0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2013-11-18  2:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Nov 17, 2013 at 02:20:50AM +0100, Marek Vasut wrote:
> The AHCI-IMX driver now supports i.MX53 as well. Add DT binding.

I think this patch can be merged independently without the driver
changes in place.  Also, I understand that the term 'binding' is
referred as binding doc, while what you're patching here is device tree
source.

Shawn

> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> ---
>  arch/arm/boot/dts/imx53.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> index cffdda2..0e92af0 100644
> --- a/arch/arm/boot/dts/imx53.dtsi
> +++ b/arch/arm/boot/dts/imx53.dtsi
> @@ -85,6 +85,15 @@
>  		interrupt-parent = <&tzic>;
>  		ranges;
>  
> +		sata: sata at 10000000 {
> +			compatible = "fsl,imx53-ahci";
> +			reg = <0x10000000 0x1000>;
> +			interrupts = <28>;
> +			clocks =  <&clks 173>, <&clks 188>, <&clks 5>;
> +			clock-names = "sata_gate", "sata_phy", "ahb";
> +			status = "disabled";
> +		};
> +
>  		ipu: ipu at 18000000 {
>  			#crtc-cells = <1>;
>  			compatible = "fsl,imx53-ipu";
> -- 
> 1.8.4.2
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 5/5] ARM: dts: imx53: Enable AHCI SATA for M53EVK
  2013-11-17  1:20   ` Marek Vasut
@ 2013-11-18  2:44     ` Shawn Guo
  -1 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2013-11-18  2:44 UTC (permalink / raw)
  To: Marek Vasut; +Cc: linux-arm-kernel, Richard Zhu, Tejun Heo, Linux-IDE

On Sun, Nov 17, 2013 at 02:20:51AM +0100, Marek Vasut wrote:
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> ---
>  arch/arm/boot/dts/imx53-m53evk.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts
> index 658d32a..c45a8d9c 100644
> --- a/arch/arm/boot/dts/imx53-m53evk.dts
> +++ b/arch/arm/boot/dts/imx53-m53evk.dts
> @@ -44,6 +44,10 @@
>  				};
>  			};
>  		};
> +
> +		sata@10000000 {
> +			status = "okay";
> +		};

Why not

&sata {

	status = "okay";
};

just like what other devices in this file are doing?

Shawn


^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH 5/5] ARM: dts: imx53: Enable AHCI SATA for M53EVK
@ 2013-11-18  2:44     ` Shawn Guo
  0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2013-11-18  2:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Nov 17, 2013 at 02:20:51AM +0100, Marek Vasut wrote:
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> ---
>  arch/arm/boot/dts/imx53-m53evk.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts
> index 658d32a..c45a8d9c 100644
> --- a/arch/arm/boot/dts/imx53-m53evk.dts
> +++ b/arch/arm/boot/dts/imx53-m53evk.dts
> @@ -44,6 +44,10 @@
>  				};
>  			};
>  		};
> +
> +		sata at 10000000 {
> +			status = "okay";
> +		};

Why not

&sata {

	status = "okay";
};

just like what other devices in this file are doing?

Shawn

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 4/5] ARM: dts: imx53: Add AHCI SATA binding
  2013-11-18  2:41     ` Shawn Guo
@ 2013-11-18 14:07       ` Marek Vasut
  -1 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-18 14:07 UTC (permalink / raw)
  To: Shawn Guo; +Cc: linux-arm-kernel, Richard Zhu, Tejun Heo, Linux-IDE

Dear Shawn Guo,

> On Sun, Nov 17, 2013 at 02:20:50AM +0100, Marek Vasut wrote:
> > The AHCI-IMX driver now supports i.MX53 as well. Add DT binding.
> 
> I think this patch can be merged independently without the driver
> changes in place.  Also, I understand that the term 'binding' is
> referred as binding doc, while what you're patching here is device tree
> source.

Ah right, shall I change it?

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH 4/5] ARM: dts: imx53: Add AHCI SATA binding
@ 2013-11-18 14:07       ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-18 14:07 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Shawn Guo,

> On Sun, Nov 17, 2013 at 02:20:50AM +0100, Marek Vasut wrote:
> > The AHCI-IMX driver now supports i.MX53 as well. Add DT binding.
> 
> I think this patch can be merged independently without the driver
> changes in place.  Also, I understand that the term 'binding' is
> referred as binding doc, while what you're patching here is device tree
> source.

Ah right, shall I change it?

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
  2013-11-17  1:20 ` Marek Vasut
@ 2013-11-18 18:47   ` Eric Nelson
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Nelson @ 2013-11-18 18:47 UTC (permalink / raw)
  To: Marek Vasut, linux-arm-kernel
  Cc: Richard Zhu, Tejun Heo, Shawn Guo, Linux-IDE

Hi Marek,

On 11/16/2013 06:20 PM, Marek Vasut wrote:
> The same code for enabling and disabling SATA clock was found in multiple
> places in the driver. Implement functions that enable/disable the SATA clock
> and use them in such places instead of duplicating the code.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> ---
>   drivers/ata/ahci_imx.c | 133 ++++++++++++++++++++++++++++---------------------
>   1 file changed, 75 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index ae2d73f..c7ee505 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
>   module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
>   MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support, 1=support)");
>
> <snip>
 >

I haven't traced through all of this, but if you're copying from
the Freescale 3.0.35 kernel, note that there's a bug in it, and
the 0x7FFFFFFD really wanted to be an 0x7FFFFFFF.

The way I read this comment, the writes need to happen in two
steps:
	- write everything with the PHY disabled
	- enable the PHY

We had reports of stalls waiting for SATA drives to be enumerated
that were solved with this commit...

	https://github.com/boundarydevices/linux-imx6/commit/0186ea224ce6bd1cb4757a0f83b0090e26a021f4

> +	/*
> +	 * set PHY Paremeters, two steps to configure the GPR13,
> +	 * one write for rest of parameters, mask of first write
> +	 * is 0x07fffffd, and the other one write for setting
> +	 * the mpll_clk_en.
> +	 */
> +	if (config) {
> +		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
> +				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
> +				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
> +				| IMX6Q_GPR13_SATA_TX_LVL_MASK
> +				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
> +				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
> +				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
> +				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
> +				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
> +	}
> +	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> +

Regards,


Eric


^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
@ 2013-11-18 18:47   ` Eric Nelson
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Nelson @ 2013-11-18 18:47 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marek,

On 11/16/2013 06:20 PM, Marek Vasut wrote:
> The same code for enabling and disabling SATA clock was found in multiple
> places in the driver. Implement functions that enable/disable the SATA clock
> and use them in such places instead of duplicating the code.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> ---
>   drivers/ata/ahci_imx.c | 133 ++++++++++++++++++++++++++++---------------------
>   1 file changed, 75 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index ae2d73f..c7ee505 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
>   module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
>   MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support, 1=support)");
>
> <snip>
 >

I haven't traced through all of this, but if you're copying from
the Freescale 3.0.35 kernel, note that there's a bug in it, and
the 0x7FFFFFFD really wanted to be an 0x7FFFFFFF.

The way I read this comment, the writes need to happen in two
steps:
	- write everything with the PHY disabled
	- enable the PHY

We had reports of stalls waiting for SATA drives to be enumerated
that were solved with this commit...

	https://github.com/boundarydevices/linux-imx6/commit/0186ea224ce6bd1cb4757a0f83b0090e26a021f4

> +	/*
> +	 * set PHY Paremeters, two steps to configure the GPR13,
> +	 * one write for rest of parameters, mask of first write
> +	 * is 0x07fffffd, and the other one write for setting
> +	 * the mpll_clk_en.
> +	 */
> +	if (config) {
> +		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
> +				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
> +				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
> +				| IMX6Q_GPR13_SATA_TX_LVL_MASK
> +				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
> +				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
> +				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
> +				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
> +				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
> +	}
> +	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> +

Regards,


Eric

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
  2013-11-18 18:47   ` Eric Nelson
@ 2013-11-18 20:23     ` Marek Vasut
  -1 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-18 20:23 UTC (permalink / raw)
  To: Eric Nelson
  Cc: linux-arm-kernel, Richard Zhu, Tejun Heo, Shawn Guo, Linux-IDE

Hi Eric,

> Hi Marek,
> 
> On 11/16/2013 06:20 PM, Marek Vasut wrote:
> > The same code for enabling and disabling SATA clock was found in multiple
> > places in the driver. Implement functions that enable/disable the SATA
> > clock and use them in such places instead of duplicating the code.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Richard Zhu <r65037@freescale.com>
> > Cc: Tejun Heo <tj@kernel.org>
> > Cc: Linux-IDE <linux-ide@vger.kernel.org>
> > ---
> > 
> >   drivers/ata/ahci_imx.c | 133
> >   ++++++++++++++++++++++++++++--------------------- 1 file changed, 75
> >   insertions(+), 58 deletions(-)
> > 
> > diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> > index ae2d73f..c7ee505 100644
> > --- a/drivers/ata/ahci_imx.c
> > +++ b/drivers/ata/ahci_imx.c
> > @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
> > 
> >   module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
> >   MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support,
> >   1=support)");
> > 
> > <snip>
> 
> I haven't traced through all of this, but if you're copying from
> the Freescale 3.0.35 kernel, note that there's a bug in it, and
> the 0x7FFFFFFD really wanted to be an 0x7FFFFFFF.

I'm not taking this from FSL 3.0.35, no, but thanks for pointing this out!

> The way I read this comment, the writes need to happen in two
> steps:
> 	- write everything with the PHY disabled
> 	- enable the PHY
> 
> We had reports of stalls waiting for SATA drives to be enumerated
> that were solved with this commit...
> 
> 	https://github.com/boundarydevices/linux-
imx6/commit/0186ea224ce6bd1cb4757
> a0f83b0090e26a021f4

[...]

> > +	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> > +			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> > +			IMX6Q_GPR13_SATA_MPLL_CLK_EN);

Isn't this snippet doing exactly what your patch does ?

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
@ 2013-11-18 20:23     ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-18 20:23 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Eric,

> Hi Marek,
> 
> On 11/16/2013 06:20 PM, Marek Vasut wrote:
> > The same code for enabling and disabling SATA clock was found in multiple
> > places in the driver. Implement functions that enable/disable the SATA
> > clock and use them in such places instead of duplicating the code.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Richard Zhu <r65037@freescale.com>
> > Cc: Tejun Heo <tj@kernel.org>
> > Cc: Linux-IDE <linux-ide@vger.kernel.org>
> > ---
> > 
> >   drivers/ata/ahci_imx.c | 133
> >   ++++++++++++++++++++++++++++--------------------- 1 file changed, 75
> >   insertions(+), 58 deletions(-)
> > 
> > diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> > index ae2d73f..c7ee505 100644
> > --- a/drivers/ata/ahci_imx.c
> > +++ b/drivers/ata/ahci_imx.c
> > @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
> > 
> >   module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
> >   MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support,
> >   1=support)");
> > 
> > <snip>
> 
> I haven't traced through all of this, but if you're copying from
> the Freescale 3.0.35 kernel, note that there's a bug in it, and
> the 0x7FFFFFFD really wanted to be an 0x7FFFFFFF.

I'm not taking this from FSL 3.0.35, no, but thanks for pointing this out!

> The way I read this comment, the writes need to happen in two
> steps:
> 	- write everything with the PHY disabled
> 	- enable the PHY
> 
> We had reports of stalls waiting for SATA drives to be enumerated
> that were solved with this commit...
> 
> 	https://github.com/boundarydevices/linux-
imx6/commit/0186ea224ce6bd1cb4757
> a0f83b0090e26a021f4

[...]

> > +	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> > +			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> > +			IMX6Q_GPR13_SATA_MPLL_CLK_EN);

Isn't this snippet doing exactly what your patch does ?

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
  2013-11-18 20:23     ` Marek Vasut
@ 2013-11-18 22:11       ` Eric Nelson
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Nelson @ 2013-11-18 22:11 UTC (permalink / raw)
  To: Marek Vasut
  Cc: linux-arm-kernel, Richard Zhu, Tejun Heo, Shawn Guo, Linux-IDE

Hi Marek,

On 11/18/2013 01:23 PM, Marek Vasut wrote:
> Hi Eric,
>
>> Hi Marek,
>>
>> On 11/16/2013 06:20 PM, Marek Vasut wrote:
>>> The same code for enabling and disabling SATA clock was found in multiple
>>> places in the driver. Implement functions that enable/disable the SATA
>>> clock and use them in such places instead of duplicating the code.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Shawn Guo <shawn.guo@linaro.org>
>>> Cc: Richard Zhu <r65037@freescale.com>
>>> Cc: Tejun Heo <tj@kernel.org>
>>> Cc: Linux-IDE <linux-ide@vger.kernel.org>
>>> ---
>>>
>>>    drivers/ata/ahci_imx.c | 133
>>>    ++++++++++++++++++++++++++++--------------------- 1 file changed, 75
>>>    insertions(+), 58 deletions(-)
>>>
>>> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
>>> index ae2d73f..c7ee505 100644
>>> --- a/drivers/ata/ahci_imx.c
>>> +++ b/drivers/ata/ahci_imx.c
>>> @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
>>>
>>>    module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
>>>    MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support,
>>>    1=support)");
>>>
>>> <snip>
>>
>> I haven't traced through all of this, but if you're copying from
>> the Freescale 3.0.35 kernel, note that there's a bug in it, and
>> the 0x7FFFFFFD really wanted to be an 0x7FFFFFFF.
>
> I'm not taking this from FSL 3.0.35, no, but thanks for pointing this out!
>
>> The way I read this comment, the writes need to happen in two
>> steps:
>> 	- write everything with the PHY disabled
>> 	- enable the PHY
>>
>> We had reports of stalls waiting for SATA drives to be enumerated
>> that were solved with this commit...
>>
>> 	https://github.com/boundarydevices/linux-
> imx6/commit/0186ea224ce6bd1cb4757
>> a0f83b0090e26a021f4
>
> [...]
>
>>> +	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
>>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
>>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
>
> Isn't this snippet doing exactly what your patch does ?
>
This part is doing the "set the bit" part:
	https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.0.35_4.1.0/arch/arm/mach-mx6/board-mx6_nitrogen6x.c#L728

The previous block isn't clearing the enable bit though:

+		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
+				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
+				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
+				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
+				| IMX6Q_GPR13_SATA_MPLL_SS_EN
+				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
+				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
+				| IMX6Q_GPR13_SATA_TX_LVL_MASK
+				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
+				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
+				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
+				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
+				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
+				| IMX6Q_GPR13_SATA_MPLL_SS_EN
+				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
+				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
+				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);

The explicit clearing of bit 1 is what was necessary (and I think
it's what the original author was after).

This was a hard one to find, because it seems that it only shows
up on some boards, and most of the time on those boards.

Here are some references:
	http://boundarydevices.com/freescale-ubuntu-image-with-4-0-0-kernel/#comment-60464
	http://boundarydevices.com/debian-installer-on-i-mx6-boards/#comment-76203

The symptom is (was) that U-Boot worked 100% of the time, but
the kernel failed to find the drive.

Using 'mm' in U-Boot to clear the bit before kernel launch also
allowed things to function.

Regards,


Eric

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
@ 2013-11-18 22:11       ` Eric Nelson
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Nelson @ 2013-11-18 22:11 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marek,

On 11/18/2013 01:23 PM, Marek Vasut wrote:
> Hi Eric,
>
>> Hi Marek,
>>
>> On 11/16/2013 06:20 PM, Marek Vasut wrote:
>>> The same code for enabling and disabling SATA clock was found in multiple
>>> places in the driver. Implement functions that enable/disable the SATA
>>> clock and use them in such places instead of duplicating the code.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Shawn Guo <shawn.guo@linaro.org>
>>> Cc: Richard Zhu <r65037@freescale.com>
>>> Cc: Tejun Heo <tj@kernel.org>
>>> Cc: Linux-IDE <linux-ide@vger.kernel.org>
>>> ---
>>>
>>>    drivers/ata/ahci_imx.c | 133
>>>    ++++++++++++++++++++++++++++--------------------- 1 file changed, 75
>>>    insertions(+), 58 deletions(-)
>>>
>>> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
>>> index ae2d73f..c7ee505 100644
>>> --- a/drivers/ata/ahci_imx.c
>>> +++ b/drivers/ata/ahci_imx.c
>>> @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
>>>
>>>    module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
>>>    MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support,
>>>    1=support)");
>>>
>>> <snip>
>>
>> I haven't traced through all of this, but if you're copying from
>> the Freescale 3.0.35 kernel, note that there's a bug in it, and
>> the 0x7FFFFFFD really wanted to be an 0x7FFFFFFF.
>
> I'm not taking this from FSL 3.0.35, no, but thanks for pointing this out!
>
>> The way I read this comment, the writes need to happen in two
>> steps:
>> 	- write everything with the PHY disabled
>> 	- enable the PHY
>>
>> We had reports of stalls waiting for SATA drives to be enumerated
>> that were solved with this commit...
>>
>> 	https://github.com/boundarydevices/linux-
> imx6/commit/0186ea224ce6bd1cb4757
>> a0f83b0090e26a021f4
>
> [...]
>
>>> +	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
>>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
>>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
>
> Isn't this snippet doing exactly what your patch does ?
>
This part is doing the "set the bit" part:
	https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.0.35_4.1.0/arch/arm/mach-mx6/board-mx6_nitrogen6x.c#L728

The previous block isn't clearing the enable bit though:

+		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
+				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
+				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
+				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
+				| IMX6Q_GPR13_SATA_MPLL_SS_EN
+				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
+				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
+				| IMX6Q_GPR13_SATA_TX_LVL_MASK
+				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
+				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
+				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
+				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
+				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
+				| IMX6Q_GPR13_SATA_MPLL_SS_EN
+				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
+				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
+				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);

The explicit clearing of bit 1 is what was necessary (and I think
it's what the original author was after).

This was a hard one to find, because it seems that it only shows
up on some boards, and most of the time on those boards.

Here are some references:
	http://boundarydevices.com/freescale-ubuntu-image-with-4-0-0-kernel/#comment-60464
	http://boundarydevices.com/debian-installer-on-i-mx6-boards/#comment-76203

The symptom is (was) that U-Boot worked 100% of the time, but
the kernel failed to find the drive.

Using 'mm' in U-Boot to clear the bit before kernel launch also
allowed things to function.

Regards,


Eric

^ permalink raw reply	[flat|nested] 30+ messages in thread
* RE: [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
  2013-11-18 22:11       ` Eric Nelson
@ 2013-11-20  4:29         ` Richard Zhu
  -1 siblings, 0 replies; 30+ messages in thread
From: Richard Zhu @ 2013-11-20  4:29 UTC (permalink / raw)
  To: Eric Nelson, Marek Vasut
  Cc: linux-arm-kernel, Tejun Heo, Shawn Guo, Linux-IDE

Hi Nelson:

> -----Original Message-----
> From: linux-ide-owner@vger.kernel.org [mailto:linux-ide-owner@vger.kernel.org]
> On Behalf Of Eric Nelson
> Sent: Tuesday, November 19, 2013 6:12 AM
> To: Marek Vasut
> Cc: linux-arm-kernel@lists.infradead.org; Zhu Richard-R65037; Tejun Heo; Shawn
> Guo; Linux-IDE
> Subject: Re: [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
> 
> Hi Marek,
> 
> On 11/18/2013 01:23 PM, Marek Vasut wrote:
> > Hi Eric,
> >
> >> Hi Marek,
> >>
> >> On 11/16/2013 06:20 PM, Marek Vasut wrote:
> >>> The same code for enabling and disabling SATA clock was found in
> >>> multiple places in the driver. Implement functions that
> >>> enable/disable the SATA clock and use them in such places instead of
> duplicating the code.
> >>>
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> Cc: Shawn Guo <shawn.guo@linaro.org>
> >>> Cc: Richard Zhu <r65037@freescale.com>
> >>> Cc: Tejun Heo <tj@kernel.org>
> >>> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> >>> ---
> >>>
> >>>    drivers/ata/ahci_imx.c | 133
> >>>    ++++++++++++++++++++++++++++--------------------- 1 file changed, 75
> >>>    insertions(+), 58 deletions(-)
> >>>
> >>> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index
> >>> ae2d73f..c7ee505 100644
> >>> --- a/drivers/ata/ahci_imx.c
> >>> +++ b/drivers/ata/ahci_imx.c
> >>> @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
> >>>
> >>>    module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
> >>>    MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support,
> >>>    1=support)");
> >>>
> >>> <snip>
> >>
> >> I haven't traced through all of this, but if you're copying from the
> >> Freescale 3.0.35 kernel, note that there's a bug in it, and the
> >> 0x7FFFFFFD really wanted to be an 0x7FFFFFFF.
> >
> > I'm not taking this from FSL 3.0.35, no, but thanks for pointing this out!
> >
> >> The way I read this comment, the writes need to happen in two
> >> steps:
> >> 	- write everything with the PHY disabled
> >> 	- enable the PHY
> >>
> >> We had reports of stalls waiting for SATA drives to be enumerated
> >> that were solved with this commit...
> >>
> >> 	https://github.com/boundarydevices/linux-
> > imx6/commit/0186ea224ce6bd1cb4757
> >> a0f83b0090e26a021f4
> >
> > [...]
> >
> >>> +	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> >>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> >>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> >
> > Isn't this snippet doing exactly what your patch does ?
> >
> This part is doing the "set the bit" part:
> 	https://github.com/boundarydevices/linux-imx6/blob/boundary-
> imx_3.0.35_4.1.0/arch/arm/mach-mx6/board-mx6_nitrogen6x.c#L728
> 
> The previous block isn't clearing the enable bit though:
> 
> +		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
> +				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
> +				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
> +				| IMX6Q_GPR13_SATA_TX_LVL_MASK
> +				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
> +				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
> +				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
> +				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
> +				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
> 
> The explicit clearing of bit 1 is what was necessary (and I think it's what
> the original author was after).
> 
> This was a hard one to find, because it seems that it only shows up on some
> boards, and most of the time on those boards.
> 
> Here are some references:
> 	http://boundarydevices.com/freescale-ubuntu-image-with-4-0-0-
> kernel/#comment-60464
> 	http://boundarydevices.com/debian-installer-on-i-mx6-boards/#comment-
> 76203
> 
> The symptom is (was) that U-Boot worked 100% of the time, but the kernel
> failed to find the drive.
> 
> Using 'mm' in U-Boot to clear the bit before kernel launch also allowed things
> to function.
> 
[Richard] Agree to explicit clearing of bit1, refer to the chapter "Power-Up Sequences" of 
"Serial Advanced Technology Attachment PHY (SATA PHY)" in iMX6 RM doc.

> Regards,
> 
> 
> Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Best Regards
Richard Zhu


^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
@ 2013-11-20  4:29         ` Richard Zhu
  0 siblings, 0 replies; 30+ messages in thread
From: Richard Zhu @ 2013-11-20  4:29 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Nelson:

> -----Original Message-----
> From: linux-ide-owner at vger.kernel.org [mailto:linux-ide-owner at vger.kernel.org]
> On Behalf Of Eric Nelson
> Sent: Tuesday, November 19, 2013 6:12 AM
> To: Marek Vasut
> Cc: linux-arm-kernel at lists.infradead.org; Zhu Richard-R65037; Tejun Heo; Shawn
> Guo; Linux-IDE
> Subject: Re: [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
> 
> Hi Marek,
> 
> On 11/18/2013 01:23 PM, Marek Vasut wrote:
> > Hi Eric,
> >
> >> Hi Marek,
> >>
> >> On 11/16/2013 06:20 PM, Marek Vasut wrote:
> >>> The same code for enabling and disabling SATA clock was found in
> >>> multiple places in the driver. Implement functions that
> >>> enable/disable the SATA clock and use them in such places instead of
> duplicating the code.
> >>>
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> Cc: Shawn Guo <shawn.guo@linaro.org>
> >>> Cc: Richard Zhu <r65037@freescale.com>
> >>> Cc: Tejun Heo <tj@kernel.org>
> >>> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> >>> ---
> >>>
> >>>    drivers/ata/ahci_imx.c | 133
> >>>    ++++++++++++++++++++++++++++--------------------- 1 file changed, 75
> >>>    insertions(+), 58 deletions(-)
> >>>
> >>> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index
> >>> ae2d73f..c7ee505 100644
> >>> --- a/drivers/ata/ahci_imx.c
> >>> +++ b/drivers/ata/ahci_imx.c
> >>> @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
> >>>
> >>>    module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
> >>>    MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support,
> >>>    1=support)");
> >>>
> >>> <snip>
> >>
> >> I haven't traced through all of this, but if you're copying from the
> >> Freescale 3.0.35 kernel, note that there's a bug in it, and the
> >> 0x7FFFFFFD really wanted to be an 0x7FFFFFFF.
> >
> > I'm not taking this from FSL 3.0.35, no, but thanks for pointing this out!
> >
> >> The way I read this comment, the writes need to happen in two
> >> steps:
> >> 	- write everything with the PHY disabled
> >> 	- enable the PHY
> >>
> >> We had reports of stalls waiting for SATA drives to be enumerated
> >> that were solved with this commit...
> >>
> >> 	https://github.com/boundarydevices/linux-
> > imx6/commit/0186ea224ce6bd1cb4757
> >> a0f83b0090e26a021f4
> >
> > [...]
> >
> >>> +	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> >>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> >>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> >
> > Isn't this snippet doing exactly what your patch does ?
> >
> This part is doing the "set the bit" part:
> 	https://github.com/boundarydevices/linux-imx6/blob/boundary-
> imx_3.0.35_4.1.0/arch/arm/mach-mx6/board-mx6_nitrogen6x.c#L728
> 
> The previous block isn't clearing the enable bit though:
> 
> +		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
> +				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
> +				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
> +				| IMX6Q_GPR13_SATA_TX_LVL_MASK
> +				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
> +				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
> +				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
> +				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
> +				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
> 
> The explicit clearing of bit 1 is what was necessary (and I think it's what
> the original author was after).
> 
> This was a hard one to find, because it seems that it only shows up on some
> boards, and most of the time on those boards.
> 
> Here are some references:
> 	http://boundarydevices.com/freescale-ubuntu-image-with-4-0-0-
> kernel/#comment-60464
> 	http://boundarydevices.com/debian-installer-on-i-mx6-boards/#comment-
> 76203
> 
> The symptom is (was) that U-Boot worked 100% of the time, but the kernel
> failed to find the drive.
> 
> Using 'mm' in U-Boot to clear the bit before kernel launch also allowed things
> to function.
> 
[Richard] Agree to explicit clearing of bit1, refer to the chapter "Power-Up Sequences" of 
"Serial Advanced Technology Attachment PHY (SATA PHY)" in iMX6 RM doc.

> Regards,
> 
> 
> Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in the
> body of a message to majordomo at vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Best Regards
Richard Zhu

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
  2013-11-18 22:11       ` Eric Nelson
@ 2013-11-20  9:55         ` Marek Vasut
  -1 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-20  9:55 UTC (permalink / raw)
  To: Eric Nelson
  Cc: linux-arm-kernel, Richard Zhu, Tejun Heo, Shawn Guo, Linux-IDE

Dear Eric Nelson,

> Hi Marek,
> 
> On 11/18/2013 01:23 PM, Marek Vasut wrote:
> > Hi Eric,
> > 
> >> Hi Marek,
> >> 
> >> On 11/16/2013 06:20 PM, Marek Vasut wrote:
> >>> The same code for enabling and disabling SATA clock was found in
> >>> multiple places in the driver. Implement functions that enable/disable
> >>> the SATA clock and use them in such places instead of duplicating the
> >>> code.
> >>> 
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> Cc: Shawn Guo <shawn.guo@linaro.org>
> >>> Cc: Richard Zhu <r65037@freescale.com>
> >>> Cc: Tejun Heo <tj@kernel.org>
> >>> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> >>> ---
> >>> 
> >>>    drivers/ata/ahci_imx.c | 133
> >>>    ++++++++++++++++++++++++++++--------------------- 1 file changed, 75
> >>>    insertions(+), 58 deletions(-)
> >>> 
> >>> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> >>> index ae2d73f..c7ee505 100644
> >>> --- a/drivers/ata/ahci_imx.c
> >>> +++ b/drivers/ata/ahci_imx.c
> >>> @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
> >>> 
> >>>    module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
> >>>    MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't
> >>>    support, 1=support)");
> >>> 
> >>> <snip>
> >> 
> >> I haven't traced through all of this, but if you're copying from
> >> the Freescale 3.0.35 kernel, note that there's a bug in it, and
> >> the 0x7FFFFFFD really wanted to be an 0x7FFFFFFF.
> > 
> > I'm not taking this from FSL 3.0.35, no, but thanks for pointing this
> > out!
> > 
> >> The way I read this comment, the writes need to happen in two
> >> 
> >> steps:
> >> 	- write everything with the PHY disabled
> >> 	- enable the PHY
> >> 
> >> We had reports of stalls waiting for SATA drives to be enumerated
> >> that were solved with this commit...
> >> 
> >> 	https://github.com/boundarydevices/linux-
> > 
> > imx6/commit/0186ea224ce6bd1cb4757
> > 
> >> a0f83b0090e26a021f4
> > 
> > [...]
> > 
> >>> +	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> >>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> >>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> > 
> > Isn't this snippet doing exactly what your patch does ?
> 
> This part is doing the "set the bit" part:
> 	https://github.com/boundarydevices/linux-imx6/blob/boundary-
imx_3.0.35_4.1
> .0/arch/arm/mach-mx6/board-mx6_nitrogen6x.c#L728
> 
> The previous block isn't clearing the enable bit though:
> 
> +		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
> +				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
> +				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
> +				| IMX6Q_GPR13_SATA_TX_LVL_MASK
> +				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
> +				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
> +				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
> +				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
> +				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
> 
> The explicit clearing of bit 1 is what was necessary (and I think
> it's what the original author was after).
> 
> This was a hard one to find, because it seems that it only shows
> up on some boards, and most of the time on those boards.
> 
> Here are some references:
> 	http://boundarydevices.com/freescale-ubuntu-image-with-4-0-0-
kernel/#comme
> nt-60464
> http://boundarydevices.com/debian-installer-on-i-mx6-boards/#comment-76203
> 
> The symptom is (was) that U-Boot worked 100% of the time, but
> the kernel failed to find the drive.
> 
> Using 'mm' in U-Boot to clear the bit before kernel launch also
> allowed things to function.

Ah, now I see it. I will cook a patch, thanks!

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls
@ 2013-11-20  9:55         ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-20  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Eric Nelson,

> Hi Marek,
> 
> On 11/18/2013 01:23 PM, Marek Vasut wrote:
> > Hi Eric,
> > 
> >> Hi Marek,
> >> 
> >> On 11/16/2013 06:20 PM, Marek Vasut wrote:
> >>> The same code for enabling and disabling SATA clock was found in
> >>> multiple places in the driver. Implement functions that enable/disable
> >>> the SATA clock and use them in such places instead of duplicating the
> >>> code.
> >>> 
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> Cc: Shawn Guo <shawn.guo@linaro.org>
> >>> Cc: Richard Zhu <r65037@freescale.com>
> >>> Cc: Tejun Heo <tj@kernel.org>
> >>> Cc: Linux-IDE <linux-ide@vger.kernel.org>
> >>> ---
> >>> 
> >>>    drivers/ata/ahci_imx.c | 133
> >>>    ++++++++++++++++++++++++++++--------------------- 1 file changed, 75
> >>>    insertions(+), 58 deletions(-)
> >>> 
> >>> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> >>> index ae2d73f..c7ee505 100644
> >>> --- a/drivers/ata/ahci_imx.c
> >>> +++ b/drivers/ata/ahci_imx.c
> >>> @@ -47,6 +47,73 @@ static int ahci_imx_hotplug;
> >>> 
> >>>    module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
> >>>    MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't
> >>>    support, 1=support)");
> >>> 
> >>> <snip>
> >> 
> >> I haven't traced through all of this, but if you're copying from
> >> the Freescale 3.0.35 kernel, note that there's a bug in it, and
> >> the 0x7FFFFFFD really wanted to be an 0x7FFFFFFF.
> > 
> > I'm not taking this from FSL 3.0.35, no, but thanks for pointing this
> > out!
> > 
> >> The way I read this comment, the writes need to happen in two
> >> 
> >> steps:
> >> 	- write everything with the PHY disabled
> >> 	- enable the PHY
> >> 
> >> We had reports of stalls waiting for SATA drives to be enumerated
> >> that were solved with this commit...
> >> 
> >> 	https://github.com/boundarydevices/linux-
> > 
> > imx6/commit/0186ea224ce6bd1cb4757
> > 
> >> a0f83b0090e26a021f4
> > 
> > [...]
> > 
> >>> +	regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> >>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> >>> +			IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> > 
> > Isn't this snippet doing exactly what your patch does ?
> 
> This part is doing the "set the bit" part:
> 	https://github.com/boundarydevices/linux-imx6/blob/boundary-
imx_3.0.35_4.1
> .0/arch/arm/mach-mx6/board-mx6_nitrogen6x.c#L728
> 
> The previous block isn't clearing the enable bit though:
> 
> +		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +				IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
> +				| IMX6Q_GPR13_SATA_SPD_MODE_MASK
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_MASK
> +				| IMX6Q_GPR13_SATA_TX_BOOST_MASK
> +				| IMX6Q_GPR13_SATA_TX_LVL_MASK
> +				| IMX6Q_GPR13_SATA_TX_EDGE_RATE
> +				, IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
> +				| IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
> +				| IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
> +				| IMX6Q_GPR13_SATA_SPD_MODE_3P0G
> +				| IMX6Q_GPR13_SATA_MPLL_SS_EN
> +				| IMX6Q_GPR13_SATA_TX_ATTEN_9_16
> +				| IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
> +				| IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
> 
> The explicit clearing of bit 1 is what was necessary (and I think
> it's what the original author was after).
> 
> This was a hard one to find, because it seems that it only shows
> up on some boards, and most of the time on those boards.
> 
> Here are some references:
> 	http://boundarydevices.com/freescale-ubuntu-image-with-4-0-0-
kernel/#comme
> nt-60464
> http://boundarydevices.com/debian-installer-on-i-mx6-boards/#comment-76203
> 
> The symptom is (was) that U-Boot worked 100% of the time, but
> the kernel failed to find the drive.
> 
> Using 'mm' in U-Boot to clear the bit before kernel launch also
> allowed things to function.

Ah, now I see it. I will cook a patch, thanks!

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH 5/5] ARM: dts: imx53: Enable AHCI SATA for M53EVK
  2013-11-17 22:56 [PATCH V2 " Marek Vasut
@ 2013-11-17 22:56   ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17 22:56 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Marek Vasut, Shawn Guo, Richard Zhu, Tejun Heo, Linux-IDE

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 arch/arm/boot/dts/imx53-m53evk.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts
index 658d32a..c45a8d9c 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -44,6 +44,10 @@
 				};
 			};
 		};
+
+		sata@10000000 {
+			status = "okay";
+		};
 	};
 
 	backlight {
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 5/5] ARM: dts: imx53: Enable AHCI SATA for M53EVK
@ 2013-11-17 22:56   ` Marek Vasut
  0 siblings, 0 replies; 30+ messages in thread
From: Marek Vasut @ 2013-11-17 22:56 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Linux-IDE <linux-ide@vger.kernel.org>
---
 arch/arm/boot/dts/imx53-m53evk.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts
index 658d32a..c45a8d9c 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -44,6 +44,10 @@
 				};
 			};
 		};
+
+		sata at 10000000 {
+			status = "okay";
+		};
 	};
 
 	backlight {
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 30+ messages in thread
end of thread, other threads:[~2013-11-20  9:56 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-17  1:20 [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls Marek Vasut
2013-11-17  1:20 ` Marek Vasut
2013-11-17  1:20 ` [PATCH 2/5] ahci: imx: Add i.MX53 support Marek Vasut
2013-11-17  1:20   ` Marek Vasut
2013-11-17  1:20 ` [PATCH 3/5] ARM: imx: imx53: Add SATA PHY clock Marek Vasut
2013-11-17  1:20   ` Marek Vasut
2013-11-17  1:20 ` [PATCH 4/5] ARM: dts: imx53: Add AHCI SATA binding Marek Vasut
2013-11-17  1:20   ` Marek Vasut
2013-11-18  2:41   ` Shawn Guo
2013-11-18  2:41     ` Shawn Guo
2013-11-18 14:07     ` Marek Vasut
2013-11-18 14:07       ` Marek Vasut
2013-11-17  1:20 ` [PATCH 5/5] ARM: dts: imx53: Enable AHCI SATA for M53EVK Marek Vasut
2013-11-17  1:20   ` Marek Vasut
2013-11-18  2:44   ` Shawn Guo
2013-11-18  2:44     ` Shawn Guo
2013-11-17  8:15 ` [PATCH 1/5] ahci: imx: Pull out the clock enable/disable calls Lothar Waßmann
2013-11-17  8:15   ` Lothar Waßmann
2013-11-18 18:47 ` Eric Nelson
2013-11-18 18:47   ` Eric Nelson
2013-11-18 20:23   ` Marek Vasut
2013-11-18 20:23     ` Marek Vasut
2013-11-18 22:11     ` Eric Nelson
2013-11-18 22:11       ` Eric Nelson
2013-11-20  4:29       ` Richard Zhu
2013-11-20  4:29         ` Richard Zhu
2013-11-20  9:55       ` Marek Vasut
2013-11-20  9:55         ` Marek Vasut
2013-11-17 22:56 [PATCH V2 " Marek Vasut
2013-11-17 22:56 ` [PATCH 5/5] ARM: dts: imx53: Enable AHCI SATA for M53EVK Marek Vasut
2013-11-17 22:56   ` Marek Vasut

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.