Skip to contentSkip to conversation

ahci: enable ahci sata support on imx6q

PATCH Superseded Richard Zhu 7 participants 23 messages Jul 1, 2013 – Jul 2, 2013

Subsystems (6)

v2 of 9 Superseded Latest is v9 →

lore
Share & tools
lore
#0 lore raw

v2 cover ahci: enable ahci sata support on imx6q

v2: add imx6q specific ahci sata support
- Setup standalone imx ahci sata driver, because of the misalignments of the bits definition of the HBA register.
- Replace the node by the label in the board dts.
-

v1: http://www.spinics.net/lists/linux-ide/msg45581.html
- add imx6q specific ahci sata support to arch/arm/mach-imx/mach-imx6q.c

These patches is based on imx/dt branch of "http://git.linaro.org/git-ro/people/shawnguo/linux-2.6.git"

[v2 1/4] ARM: dtsi: enable ahci sata on imx6q platforms [v2 2/4] imx: ahci: enable ahci sata on imx6q platforms [v2 3/4] ARM: imx6q: update the sata bits definitions of gpr13 [v2 4/4] sata: imx: add ahci sata support on imx platforms

.../devicetree/bindings/ata/ahci-platform.txt      |    2 +-
arch/arm/mach-imx/mach-imx6q.c                     |   85 +++++-
drivers/ata/Kconfig                                |    8 +
drivers/ata/Makefile                               |    1 +
drivers/ata/sata_imx.c                             |  349 ++++++++++++++++++++
include/linux/mfd/syscon/imx6q-iomuxc-gpr.h        |  121 +++++--
6 files changed, 527 insertions(+), 39 deletions(-)
#1 lore raw

PATCH v2 1/4 ARM: dtsi: enable ahci sata on imx6q platforms

Only imx6q has the ahci sata controller, enable
it on imx6q platforms.

---
 arch/arm/boot/dts/imx6q-sabreauto.dts |    4 ++++
 arch/arm/boot/dts/imx6q-sabrelite.dts |    4 ++++
 arch/arm/boot/dts/imx6q-sabresd.dts   |    4 ++++
 arch/arm/boot/dts/imx6q.dtsi          |    9 +++++++++
 4 files changed, 21 insertions(+), 0 deletions(-)
arch/arm/boot/dts/imx6q-sabreauto.dts +4 -0
index 09a7580..dd40f00 100644
@@ -20,6 +20,10 @@
 	compatible = "fsl,imx6q-sabreauto", "fsl,imx6q";
 };
 
+&ahci { /* AHCI SATA */
+	status = "okay";
+};
+
 &iomuxc {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_hog>;
arch/arm/boot/dts/imx6q-sabrelite.dts +4 -0
index 6a00066..fde2afd 100644
@@ -65,6 +65,10 @@
 	};
 };
 
+&ahci { /* AHCI SATA */
+	status = "okay";
+};
+
 &ecspi1 {
 	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 0>;
arch/arm/boot/dts/imx6q-sabresd.dts +4 -0
index 0038228..e9b6825 100644
@@ -20,6 +20,10 @@
 	compatible = "fsl,imx6q-sabresd", "fsl,imx6q";
 };
 
+&ahci { /* AHCI SATA */
+	status = "okay";
+};
+
 &iomuxc {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_hog>;
arch/arm/boot/dts/imx6q.dtsi +9 -0
index e7dd2c4..7c1af2b 100644
@@ -424,6 +424,15 @@
 			};
 		};
 
+		ahci: ahci@02200000 { /* AHCI SATA */
+			compatible = "snps,imx-ahci";
+			reg = <0x02200000 0x4000>;
+			interrupts = <0 39 0x04>;
+			clocks = <&clks 154>, <&clks 187>;
+			clock-names = "sata", "sata_ref_100m";
+			status = "disabled";
+		};
+
 		ipu2: ipu@02800000 {
 			#crtc-cells = <1>;
 			compatible = "fsl,imx6q-ipu";
Signature · 1 line
1.7.5.4
#2 lore raw

PATCH v2 1/4 ARM: dtsi: enable ahci sata on imx6q platforms

Quoted 16 lines · Richard Zhu

Only imx6q has the ahci sata controller, enable
it on imx6q platforms.

---
 arch/arm/boot/dts/imx6q-sabreauto.dts |    4 ++++
 arch/arm/boot/dts/imx6q-sabrelite.dts |    4 ++++
 arch/arm/boot/dts/imx6q-sabresd.dts   |    4 ++++
 arch/arm/boot/dts/imx6q.dtsi          |    9 +++++++++
 4 files changed, 21 insertions(+), 0 deletions(-)
arch/arm/boot/dts/imx6q-sabreauto.dts +0 -0
index 09a7580..dd40f00 100644

arch/arm/boot/dts/imx6q-sabreauto.dts line 23

Quoted 4 lines
arch/arm/boot/dts/imx6q-sabreauto.dts +1 -0
@@ -20,6 +20,10 @@
 	compatible = "fsl,imx6q-sabreauto", "fsl,imx6q";
 };
 
+&ahci { /* AHCI SATA */

All these "AHCI SATA" comments are not so useful. Please just drop them.

Shawn

Quoted 59 lines
arch/arm/boot/dts/imx6q-sabreauto.dts +3 -0
+	status = "okay";
+};
+
 &iomuxc {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_hog>;
arch/arm/boot/dts/imx6q-sabrelite.dts +4 -0
index 6a00066..fde2afd 100644
@@ -65,6 +65,10 @@
 	};
 };
 
+&ahci { /* AHCI SATA */
+	status = "okay";
+};
+
 &ecspi1 {
 	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 0>;
arch/arm/boot/dts/imx6q-sabresd.dts +4 -0
index 0038228..e9b6825 100644
@@ -20,6 +20,10 @@
 	compatible = "fsl,imx6q-sabresd", "fsl,imx6q";
 };
 
+&ahci { /* AHCI SATA */
+	status = "okay";
+};
+
 &iomuxc {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_hog>;
arch/arm/boot/dts/imx6q.dtsi +9 -0
index e7dd2c4..7c1af2b 100644
@@ -424,6 +424,15 @@
 			};
 		};
 
+		ahci: ahci@02200000 { /* AHCI SATA */
+			compatible = "snps,imx-ahci";
+			reg = <0x02200000 0x4000>;
+			interrupts = <0 39 0x04>;
+			clocks = <&clks 154>, <&clks 187>;
+			clock-names = "sata", "sata_ref_100m";
+			status = "disabled";
+		};
+
 		ipu2: ipu@02800000 {
 			#crtc-cells = <1>;
 			compatible = "fsl,imx6q-ipu";
Signature · 1 line
1.7.5.4
#3 lore raw

PATCH v2 1/4 ARM: dtsi: enable ahci sata on imx6q platforms

Hi Shawn:
Thanks for your comments.

Best Regards
Richard Zhu
________________________________________
From: Shawn Guo [shawn.guo at linaro.org]
Sent: Monday, July 01, 2013 10:37 PM
To: Richard Zhu
Cc: linux-arm-kernel at lists.infradead.org; jgarzik at pobox.com; avorontsov at ru.mvista.com; rob.herring at calxeda.com; s.hauer at pengutronix.de; linux-ide at vger.kernel.org; Zhu Richard-R65037
Subject: Re: [v2 1/4] ARM: dtsi: enable ahci sata on imx6q platforms

Quoted 16 lines · Richard Zhu

Only imx6q has the ahci sata controller, enable
it on imx6q platforms.

---
 arch/arm/boot/dts/imx6q-sabreauto.dts |    4 ++++
 arch/arm/boot/dts/imx6q-sabrelite.dts |    4 ++++
 arch/arm/boot/dts/imx6q-sabresd.dts   |    4 ++++
 arch/arm/boot/dts/imx6q.dtsi          |    9 +++++++++
 4 files changed, 21 insertions(+), 0 deletions(-)
arch/arm/boot/dts/imx6q-sabreauto.dts +0 -0
index 09a7580..dd40f00 100644

arch/arm/boot/dts/imx6q-sabreauto.dts line 23

Quoted 4 lines
arch/arm/boot/dts/imx6q-sabreauto.dts +1 -0
@@ -20,6 +20,10 @@
      compatible = "fsl,imx6q-sabreauto", "fsl,imx6q";
 };
 
+&ahci { /* AHCI SATA */

All these "AHCI SATA" comments are not so useful. Please just drop them.
[Richard] Ok, they would be removed in next version.

Shawn

Quoted 59 lines
arch/arm/boot/dts/imx6q-sabreauto.dts +3 -0
+     status = "okay";
+};
+
 &iomuxc {
      pinctrl-names = "default";
      pinctrl-0 = <&pinctrl_hog>;
arch/arm/boot/dts/imx6q-sabrelite.dts +4 -0
index 6a00066..fde2afd 100644
@@ -65,6 +65,10 @@
      };
 };
 
+&ahci { /* AHCI SATA */
+     status = "okay";
+};
+
 &ecspi1 {
      fsl,spi-num-chipselects = <1>;
      cs-gpios = <&gpio3 19 0>;
arch/arm/boot/dts/imx6q-sabresd.dts +4 -0
index 0038228..e9b6825 100644
@@ -20,6 +20,10 @@
      compatible = "fsl,imx6q-sabresd", "fsl,imx6q";
 };
 
+&ahci { /* AHCI SATA */
+     status = "okay";
+};
+
 &iomuxc {
      pinctrl-names = "default";
      pinctrl-0 = <&pinctrl_hog>;
arch/arm/boot/dts/imx6q.dtsi +9 -1
index e7dd2c4..7c1af2b 100644
@@ -424,6 +424,15 @@
                      };
              };
 
+             ahci: ahci at 02200000 { /* AHCI SATA */
+                     compatible = "snps,imx-ahci";
+                     reg = <0x02200000 0x4000>;
+                     interrupts = <0 39 0x04>;
+                     clocks = <&clks 154>, <&clks 187>;
+                     clock-names = "sata", "sata_ref_100m";
+                     status = "disabled";
+             };
+
              ipu2: ipu at 02800000 {
                      #crtc-cells = <1>;
                      compatible = "fsl,imx6q-ipu";
--

1.7.5.4

#4 lore raw

PATCH v2 2/4 imx: ahci: enable ahci sata on imx6q platforms

In reply to Richard Zhu

Only the imx6q contains the ahci sata controller,
other imx6 SoCs don't have it.

Enable the ahci sata only on imx6q platforms

---
 arch/arm/mach-imx/mach-imx6q.c |   85 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 84 insertions(+), 1 deletions(-)
arch/arm/mach-imx/mach-imx6q.c +84 -1
index 045e5e3..ab81fa3 100644
@@ -30,6 +30,7 @@
 #include <linux/regmap.h>
 #include <linux/micrel_phy.h>
 #include <linux/mfd/syscon.h>
+#include <linux/ahci_platform.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -39,6 +40,8 @@
 #include "cpuidle.h"
 #include "hardware.h"
 
+#define MX6Q_SATA_BASE_ADDR	0x02200000
+
 static u32 chip_revision;
 
 int imx6q_revision(void)
@@ -162,12 +165,92 @@ static void __init imx6q_usb_init(void)
 	imx_anatop_usb_chrg_detect_disable();
 }
 
+/* imx6q ahci module initialization. */
+static int imx6q_sata_phy_clk(struct device *dev, int enable)
+{
+	int ret = 0;
+	struct clk *sata_ref_clk;
+
+	sata_ref_clk = devm_clk_get(dev, "sata_ref_100m");
+	if (IS_ERR(sata_ref_clk)) {
+		dev_err(dev, "can't get sata_ref clock.\n");
+		return PTR_ERR(sata_ref_clk);
+	}
+	if (enable) {
+		/* Enable PHY clock */
+		ret = clk_prepare_enable(sata_ref_clk);
+		if (ret < 0) {
+			dev_err(dev, "can't prepare-enable sata_ref clock\n");
+			clk_put(sata_ref_clk);
+			ret = PTR_ERR(sata_ref_clk);
+		}
+	} else {
+		/* Disable PHY clock */
+		clk_disable_unprepare(sata_ref_clk);
+	}
+
+	return ret;
+}
+
+static int imx6q_sata_init(struct device *dev, void __iomem *addr)
+{
+	int ret = 0;
+	struct regmap *gpr;
+
+	ret = imx6q_sata_phy_clk(dev, true);
+	if (ret < 0)
+		return ret;
+
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (IS_ERR(gpr)) {
+		pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");
+		return PTR_ERR(gpr);
+	}
+
+	/*
+	 * 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(gpr, 0x34, 0x07fffffd, 0x0593e4c4);
+	regmap_update_bits(gpr, 0x34, 0x2, 0x2);
+	usleep_range(100, 200);
+
+	return 0;
+}
+
+static void imx6q_sata_exit(struct device *dev)
+{
+	struct regmap *gpr;
+
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (IS_ERR(gpr))
+		pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");
+
+	regmap_update_bits(gpr, 0x34, 0x2, 0x0);
+
+	imx6q_sata_phy_clk(dev, false);
+}
+
+static struct ahci_platform_data imx6q_sata_pdata = {
+	.init = imx6q_sata_init,
+	.exit = imx6q_sata_exit,
+};
+
+static const struct of_dev_auxdata imx6q_auxdata_lookup[] __initconst = {
+	OF_DEV_AUXDATA("snps,imx-ahci", MX6Q_SATA_BASE_ADDR, "imx-ahci",
+			&imx6q_sata_pdata),
+	{ /* sentinel */ }
+};
+
 static void __init imx6q_init_machine(void)
 {
 	if (of_machine_is_compatible("fsl,imx6q-sabrelite"))
 		imx6q_sabrelite_init();
 
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+	of_platform_populate(NULL, of_default_bus_match_table,
+			imx6q_auxdata_lookup, NULL);
 
 	imx_anatop_init();
 	imx6q_pm_init();
Signature · 1 line
1.7.5.4
maintainer
#5 lore raw

PATCH v2 2/4 imx: ahci: enable ahci sata on imx6q platforms

Quoted 9 lines · Richard Zhu

Only the imx6q contains the ahci sata controller,
other imx6 SoCs don't have it.

Enable the ahci sata only on imx6q platforms

---
 arch/arm/mach-imx/mach-imx6q.c |   85 +++++++++++++++++++++++++++++++++++++++-
arch/arm/mach-imx/mach-imx6q.c +1 -0
+/* imx6q ahci module initialization. */

arch/arm/mach-imx/mach-imx6q.c static void __init imx6q_usb_init(void) · line 178

Quoted 10 lines
arch/arm/mach-imx/mach-imx6q.c +10 -0
+static int imx6q_sata_phy_clk(struct device *dev, int enable)
+{
+	int ret = 0;
+	struct clk *sata_ref_clk;
+
+	sata_ref_clk = devm_clk_get(dev, "sata_ref_100m");
+	if (IS_ERR(sata_ref_clk)) {
+		dev_err(dev, "can't get sata_ref clock.\n");
+		return PTR_ERR(sata_ref_clk);
+	}

devm_clk_get takes a reference to the clock. That's not something you want to do each time you enable/disable a clock.

arch/arm/mach-imx/mach-imx6q.c static void __init imx6q_usb_init(void) · line 185

Quoted 7 lines
arch/arm/mach-imx/mach-imx6q.c +7 -0
+	if (enable) {
+		/* Enable PHY clock */
+		ret = clk_prepare_enable(sata_ref_clk);
+		if (ret < 0) {
+			dev_err(dev, "can't prepare-enable sata_ref clock\n");
+			clk_put(sata_ref_clk);
+			ret = PTR_ERR(sata_ref_clk);

What are you intending by converting a valid pointer to an error code?

Quoted 21 lines
arch/arm/mach-imx/mach-imx6q.c +21 -0
+		}
+	} else {
+		/* Disable PHY clock */
+		clk_disable_unprepare(sata_ref_clk);
+	}
+
+	return ret;
+}
+
+static int imx6q_sata_init(struct device *dev, void __iomem *addr)
+{
+	int ret = 0;
+	struct regmap *gpr;
+
+	ret = imx6q_sata_phy_clk(dev, true);
+	if (ret < 0)
+		return ret;
+
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (IS_ERR(gpr)) {
+		pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");

arch/arm/mach-imx/mach-imx6q.c static void __init imx6q_usb_init(void) · line 216

Quoted 10 lines
arch/arm/mach-imx/mach-imx6q.c +10 -0
+		return PTR_ERR(gpr);
+	}
+
+	/*
+	 * 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(gpr, 0x34, 0x07fffffd, 0x0593e4c4);

You are adding the register defines in the next patch. Wouldn't it make sense to use them?

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
#6 lore raw

PATCH v2 2/4 imx: ahci: enable ahci sata on imx6q platforms

Hi Sascha;
Thanks for your comments.

Best Regards
Richard Zhu
________________________________________
From: Sascha Hauer [s.hauer at pengutronix.de]
Sent: Monday, July 01, 2013 8:55 PM
To: Richard Zhu
Cc: shawn.guo at linaro.org; linux-arm-kernel at lists.infradead.org; jgarzik at pobox.com; avorontsov at ru.mvista.com; rob.herring at calxeda.com; linux-ide at vger.kernel.org; Zhu Richard-R65037
Subject: Re: [v2 2/4] imx: ahci: enable ahci sata on imx6q platforms

Quoted 9 lines · Richard Zhu

Only the imx6q contains the ahci sata controller,
other imx6 SoCs don't have it.

Enable the ahci sata only on imx6q platforms

---
 arch/arm/mach-imx/mach-imx6q.c |   85 +++++++++++++++++++++++++++++++++++++++-
arch/arm/mach-imx/mach-imx6q.c +1 -0
+/* imx6q ahci module initialization. */

arch/arm/mach-imx/mach-imx6q.c static void __init imx6q_usb_init(void) · line 178

Quoted 10 lines
arch/arm/mach-imx/mach-imx6q.c +10 -0
+static int imx6q_sata_phy_clk(struct device *dev, int enable)
+{
+     int ret = 0;
+     struct clk *sata_ref_clk;
+
+     sata_ref_clk = devm_clk_get(dev, "sata_ref_100m");
+     if (IS_ERR(sata_ref_clk)) {
+             dev_err(dev, "can't get sata_ref clock.\n");
+             return PTR_ERR(sata_ref_clk);
+     }

devm_clk_get takes a reference to the clock. That's not something you want to do each time you enable/disable a clock.
[Richard] Accepted.

arch/arm/mach-imx/mach-imx6q.c static void __init imx6q_usb_init(void) · line 185

Quoted 7 lines
arch/arm/mach-imx/mach-imx6q.c +7 -0
+     if (enable) {
+             /* Enable PHY clock */
+             ret = clk_prepare_enable(sata_ref_clk);
+             if (ret < 0) {
+                     dev_err(dev, "can't prepare-enable sata_ref clock\n");
+                     clk_put(sata_ref_clk);
+                     ret = PTR_ERR(sata_ref_clk);

What are you intending by converting a valid pointer to an error code? [Richard] Typo-mistake, would be corrected.

Quoted 21 lines
arch/arm/mach-imx/mach-imx6q.c +21 -0
+             }
+     } else {
+             /* Disable PHY clock */
+             clk_disable_unprepare(sata_ref_clk);
+     }
+
+     return ret;
+}
+
+static int imx6q_sata_init(struct device *dev, void __iomem *addr)
+{
+     int ret = 0;
+     struct regmap *gpr;
+
+     ret = imx6q_sata_phy_clk(dev, true);
+     if (ret < 0)
+             return ret;
+
+     gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+     if (IS_ERR(gpr)) {
+             pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");

arch/arm/mach-imx/mach-imx6q.c static void __init imx6q_usb_init(void) · line 216

Quoted 10 lines
arch/arm/mach-imx/mach-imx6q.c +10 -0
+             return PTR_ERR(gpr);
+     }
+
+     /*
+      * 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(gpr, 0x34, 0x07fffffd, 0x0593e4c4);

You are adding the register defines in the next patch. Wouldn't it make sense to use them?
[Richard] Ok, would be replaced by the register's definitions.

Sascha

--

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
#7 lore raw

PATCH v2 2/4 imx: ahci: enable ahci sata on imx6q platforms

In reply to Richard Zhu
Quoted 104 lines · Richard Zhu

Only the imx6q contains the ahci sata controller,
other imx6 SoCs don't have it.

Enable the ahci sata only on imx6q platforms

---
 arch/arm/mach-imx/mach-imx6q.c |   85 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 84 insertions(+), 1 deletions(-)
arch/arm/mach-imx/mach-imx6q.c +72 -0
index 045e5e3..ab81fa3 100644
@@ -30,6 +30,7 @@
 #include <linux/regmap.h>
 #include <linux/micrel_phy.h>
 #include <linux/mfd/syscon.h>
+#include <linux/ahci_platform.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -39,6 +40,8 @@
 #include "cpuidle.h"
 #include "hardware.h"
 
+#define MX6Q_SATA_BASE_ADDR	0x02200000
+
 static u32 chip_revision;
 
 int imx6q_revision(void)
@@ -162,12 +165,92 @@ static void __init imx6q_usb_init(void)
 	imx_anatop_usb_chrg_detect_disable();
 }
 
+/* imx6q ahci module initialization. */
+static int imx6q_sata_phy_clk(struct device *dev, int enable)
+{
+	int ret = 0;
+	struct clk *sata_ref_clk;
+
+	sata_ref_clk = devm_clk_get(dev, "sata_ref_100m");
+	if (IS_ERR(sata_ref_clk)) {
+		dev_err(dev, "can't get sata_ref clock.\n");
+		return PTR_ERR(sata_ref_clk);
+	}
+	if (enable) {
+		/* Enable PHY clock */
+		ret = clk_prepare_enable(sata_ref_clk);
+		if (ret < 0) {
+			dev_err(dev, "can't prepare-enable sata_ref clock\n");
+			clk_put(sata_ref_clk);
+			ret = PTR_ERR(sata_ref_clk);
+		}
+	} else {
+		/* Disable PHY clock */
+		clk_disable_unprepare(sata_ref_clk);
+	}
+
+	return ret;
+}
+
+static int imx6q_sata_init(struct device *dev, void __iomem *addr)
+{
+	int ret = 0;
+	struct regmap *gpr;
+
+	ret = imx6q_sata_phy_clk(dev, true);
+	if (ret < 0)
+		return ret;
+
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (IS_ERR(gpr)) {
+		pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");
+		return PTR_ERR(gpr);
+	}
+
+	/*
+	 * 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(gpr, 0x34, 0x07fffffd, 0x0593e4c4);
+	regmap_update_bits(gpr, 0x34, 0x2, 0x2);
+	usleep_range(100, 200);
+
+	return 0;
+}
+
+static void imx6q_sata_exit(struct device *dev)
+{
+	struct regmap *gpr;
+
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (IS_ERR(gpr))
+		pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");
+
+	regmap_update_bits(gpr, 0x34, 0x2, 0x0);
+
+	imx6q_sata_phy_clk(dev, false);
+}
+
+static struct ahci_platform_data imx6q_sata_pdata = {

arch/arm/mach-imx/mach-imx6q.c static void __init imx6q_usb_init(void) · line 246

Quoted 10 lines
arch/arm/mach-imx/mach-imx6q.c +10 -0
+	.init = imx6q_sata_init,
+	.exit = imx6q_sata_exit,
+};
+
+static const struct of_dev_auxdata imx6q_auxdata_lookup[] __initconst = {
+	OF_DEV_AUXDATA("snps,imx-ahci", MX6Q_SATA_BASE_ADDR, "imx-ahci",
+			&imx6q_sata_pdata),
+	{ /* sentinel */ }
+};
+

This is wrong. The whole point of having an imx specific ahci platform driver is to have all these setup done in that driver instead of platform code. Using auxdata is generally a sign of not-doing-the-right-thing on a recent platform with a new created driver.

Shawn

Quoted 14 lines
arch/arm/mach-imx/mach-imx6q.c +2 -1
 static void __init imx6q_init_machine(void)
 {
 	if (of_machine_is_compatible("fsl,imx6q-sabrelite"))
 		imx6q_sabrelite_init();
 
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+	of_platform_populate(NULL, of_default_bus_match_table,
+			imx6q_auxdata_lookup, NULL);
 
 	imx_anatop_init();
 	imx6q_pm_init();
Signature · 1 line
1.7.5.4
#8 lore raw

PATCH v2 3/4 ARM: imx6q: update the sata bits definitions of gpr13

In reply to Richard Zhu

Replace the SATA_PHY_# by the more readable definitons.

---
 include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  121 ++++++++++++++++++--------
 1 files changed, 84 insertions(+), 37 deletions(-)
include/linux/mfd/syscon/imx6q-iomuxc-gpr.h +84 -37
index dab34a1..55726b9 100644
@@ -279,41 +279,88 @@
 #define IMX6Q_GPR13_CAN2_STOP_REQ		BIT(29)
 #define IMX6Q_GPR13_CAN1_STOP_REQ		BIT(28)
 #define IMX6Q_GPR13_ENET_STOP_REQ		BIT(27)
-#define IMX6Q_GPR13_SATA_PHY_8_MASK		(0x7 << 24)
-#define IMX6Q_GPR13_SATA_PHY_8_0_5_DB		(0x0 << 24)
-#define IMX6Q_GPR13_SATA_PHY_8_1_0_DB		(0x1 << 24)
-#define IMX6Q_GPR13_SATA_PHY_8_1_5_DB		(0x2 << 24)
-#define IMX6Q_GPR13_SATA_PHY_8_2_0_DB		(0x3 << 24)
-#define IMX6Q_GPR13_SATA_PHY_8_2_5_DB		(0x4 << 24)
-#define IMX6Q_GPR13_SATA_PHY_8_3_0_DB		(0x5 << 24)
-#define IMX6Q_GPR13_SATA_PHY_8_3_5_DB		(0x6 << 24)
-#define IMX6Q_GPR13_SATA_PHY_8_4_0_DB		(0x7 << 24)
-#define IMX6Q_GPR13_SATA_PHY_7_MASK		(0x1f << 19)
-#define IMX6Q_GPR13_SATA_PHY_7_SATA1I		(0x10 << 19)
-#define IMX6Q_GPR13_SATA_PHY_7_SATA1M		(0x10 << 19)
-#define IMX6Q_GPR13_SATA_PHY_7_SATA1X		(0x1a << 19)
-#define IMX6Q_GPR13_SATA_PHY_7_SATA2I		(0x12 << 19)
-#define IMX6Q_GPR13_SATA_PHY_7_SATA2M		(0x12 << 19)
-#define IMX6Q_GPR13_SATA_PHY_7_SATA2X		(0x1a << 19)
-#define IMX6Q_GPR13_SATA_PHY_6_MASK		(0x7 << 16)
-#define IMX6Q_GPR13_SATA_SPEED_MASK		BIT(15)
-#define IMX6Q_GPR13_SATA_SPEED_1P5G		0x0
-#define IMX6Q_GPR13_SATA_SPEED_3P0G		BIT(15)
-#define IMX6Q_GPR13_SATA_PHY_5			BIT(14)
-#define IMX6Q_GPR13_SATA_PHY_4_MASK		(0x7 << 11)
-#define IMX6Q_GPR13_SATA_PHY_4_16_16		(0x0 << 11)
-#define IMX6Q_GPR13_SATA_PHY_4_14_16		(0x1 << 11)
-#define IMX6Q_GPR13_SATA_PHY_4_12_16		(0x2 << 11)
-#define IMX6Q_GPR13_SATA_PHY_4_10_16		(0x3 << 11)
-#define IMX6Q_GPR13_SATA_PHY_4_9_16		(0x4 << 11)
-#define IMX6Q_GPR13_SATA_PHY_4_8_16		(0x5 << 11)
-#define IMX6Q_GPR13_SATA_PHY_3_MASK		(0xf << 7)
-#define IMX6Q_GPR13_SATA_PHY_3_OFF		0x7
-#define IMX6Q_GPR13_SATA_PHY_2_MASK		(0x1f << 2)
-#define IMX6Q_GPR13_SATA_PHY_2_OFF		0x2
-#define IMX6Q_GPR13_SATA_PHY_1_MASK		(0x3 << 0)
-#define IMX6Q_GPR13_SATA_PHY_1_FAST		(0x0 << 0)
-#define IMX6Q_GPR13_SATA_PHY_1_MED		(0x1 << 0)
-#define IMX6Q_GPR13_SATA_PHY_1_SLOW		(0x2 << 0)
-
+#define IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK		(0x7 << 24)
+#define IMX6Q_GPR13_SATA_RX_EQ_VAL_0_5_DB	(0x0 << 24)
+#define IMX6Q_GPR13_SATA_RX_EQ_VAL_1_0_DB	(0x1 << 24)
+#define IMX6Q_GPR13_SATA_RX_EQ_VAL_1_5_DB	(0x2 << 24)
+#define IMX6Q_GPR13_SATA_RX_EQ_VAL_2_0_DB	(0x3 << 24)
+#define IMX6Q_GPR13_SATA_RX_EQ_VAL_2_5_DB	(0x4 << 24)
+#define IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB	(0x5 << 24)
+#define IMX6Q_GPR13_SATA_RX_EQ_VAL_3_5_DB	(0x6 << 24)
+#define IMX6Q_GPR13_SATA_RX_EQ_VAL_4_0_DB	(0x7 << 24)
+#define IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK	(0x1f << 19)
+#define IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA1I	(0x10 << 19)
+#define IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA1M	(0x10 << 19)
+#define IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA1X	(0x1a << 19)
+#define IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2I	(0x12 << 19)
+#define IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M	(0x12 << 19)
+#define IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2X	(0x1a << 19)
+#define IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK	(0x7 << 16)
+#define IMX6Q_GPR13_SATA_RX_DPLL_MODE_1P_1F	(0x0 << 16)
+#define IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_2F	(0x1 << 16)
+#define IMX6Q_GPR13_SATA_RX_DPLL_MODE_1P_4F	(0x2 << 16)
+#define IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F	(0x3 << 16)
+#define IMX6Q_GPR13_SATA_SPD_MODE_MASK		BIT(15)
+#define IMX6Q_GPR13_SATA_SPD_MODE_1P5G		0x0
+#define IMX6Q_GPR13_SATA_SPD_MODE_3P0G		BIT(15)
+#define IMX6Q_GPR13_SATA_MPLL_SS_EN		BIT(14)
+#define IMX6Q_GPR13_SATA_TX_ATTEN_MASK		(0x7 << 11)
+#define IMX6Q_GPR13_SATA_TX_ATTEN_16_16		(0x0 << 11)
+#define IMX6Q_GPR13_SATA_TX_ATTEN_14_16		(0x1 << 11)
+#define IMX6Q_GPR13_SATA_TX_ATTEN_12_16		(0x2 << 11)
+#define IMX6Q_GPR13_SATA_TX_ATTEN_10_16		(0x3 << 11)
+#define IMX6Q_GPR13_SATA_TX_ATTEN_9_16		(0x4 << 11)
+#define IMX6Q_GPR13_SATA_TX_ATTEN_8_16		(0x5 << 11)
+#define IMX6Q_GPR13_SATA_TX_BOOST_MASK		(0xf << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_0_37_DB	(0x0 << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_0_74_DB	(0x1 << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_0_74_DB	(0x2 << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_1_11_DB	(0x3 << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_1_48_DB	(0x4 << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_1_85_DB	(0x5 << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_2_22_DB	(0x6 << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_2_59_DB	(0x7 << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_2_96_DB	(0x8 << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB	(0x9 << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_3_70_DB	(0xa << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_4_07_DB	(0xb << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_4_44_DB	(0xc << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_4_81_DB	(0xd << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_5_28_DB	(0xe << 7)
+#define IMX6Q_GPR13_SATA_TX_BOOST_5_75_DB	(0xf << 7)
+#define IMX6Q_GPR13_SATA_TX_LVL_MASK		(0x1f << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_0_937_V		(0x00 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_0_947_V		(0x01 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_0_957_V		(0x02 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_0_966_V		(0x03 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_0_976_V		(0x04 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_0_986_V		(0x05 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_0_996_V		(0x06 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_005_V		(0x07 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_015_V		(0x08 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_025_V		(0x09 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_035_V		(0x0a << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_045_V		(0x0b << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_054_V		(0x0c << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_064_V		(0x0d << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_074_V		(0x0e << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_084_V		(0x0f << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_094_V		(0x10 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_104_V		(0x11 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_113_V		(0x12 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_123_V		(0x13 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_133_V		(0x14 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_143_V		(0x15 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_152_V		(0x16 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_162_V		(0x17 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_172_V		(0x18 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_182_V		(0x19 << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_191_V		(0x1a << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_201_V		(0x1b << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_211_V		(0x1c << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_221_V		(0x1d << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_230_V		(0x1e << 2)
+#define IMX6Q_GPR13_SATA_TX_LVL_1_240_V		(0x1f << 2)
+#define IMX6Q_GPR13_SATA_MPLL_CLK_EN		BIT(1)
+#define IMX6Q_GPR13_SATA_TX_EDGE_RATE		BIT(0)
 #endif /* __LINUX_IMX6Q_IOMUXC_GPR_H */
Signature · 1 line
1.7.5.4
#9 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

In reply to Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers.
Because there are some misalignments of the bits definitions of the HBA registers and the Vendor Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock.

Setup its own ahci sata driver, enable the imx6q ahci sata support, and update the ahci sata binding document.

---
 .../devicetree/bindings/ata/ahci-platform.txt      |    2 +-
 drivers/ata/Kconfig                                |    8 +
 drivers/ata/Makefile                               |    1 +
 drivers/ata/sata_imx.c                             |  349 ++++++++++++++++++++
 4 files changed, 359 insertions(+), 1 deletions(-)
 create mode 100644 drivers/ata/sata_imx.c
Documentation/devicetree/bindings/ata/ahci-platform.txt +1 -1
index b519f9b..e252620 100644
@@ -4,7 +4,7 @@ SATA nodes are defined to describe on-chip Serial ATA controllers.
 Each SATA controller should have its own node.
 
 Required properties:
-- compatible        : compatible list, contains "calxeda,hb-ahci" or "snps,spear-ahci"
+- compatible        : compatible list, contains "calxeda,hb-ahci", "snps,spear-ahci" or "snps, imx-ahci"
 - interrupts        : <interrupt mapping for SATA IRQ>
 - reg               : <registers mapping>
 
drivers/ata/Kconfig +8 -0
index a5a3ebc..893fa0b 100644
@@ -236,6 +236,14 @@ config SATA_HIGHBANK
 
 	  If unsure, say N.
 
+config SATA_IMX
+	tristate "Freescale iMX AHCI SATA support"
+	help
+	  This option enables support for the Freescale iMX SoC's
+	  onboard AHCI SATA.
+
+	  If unsure, say N.
+
 config SATA_MV
 	tristate "Marvell SATA support"
 	help
drivers/ata/Makefile +1 -0
index c04d0fd..c40b328 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
 obj-$(CONFIG_SATA_DWC)		+= sata_dwc_460ex.o
 obj-$(CONFIG_SATA_HIGHBANK)	+= sata_highbank.o libahci.o
+obj-$(CONFIG_SATA_IMX)		+= sata_imx.o libahci.o
 
 # SFF w/ custom DMA
 obj-$(CONFIG_PDC_ADMA)		+= pdc_adma.o
drivers/ata/sata_imx.c new file +349 -0

349 lines not on this page · raw message

Signature · 1 line
1.7.5.4
#10 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

Hello Richard,

Instead of writing a separate driver for the changes you mentioned in the commit message. you can just add those changes to
the platform data (pdata-> init).

Quoted 432 lines · Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers.
Because there are some misalignments of the bits definitions of the HBA registers and the Vendor Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock.

Setup its own ahci sata driver, enable the imx6q ahci sata support, and update the ahci sata binding document.

---
 .../devicetree/bindings/ata/ahci-platform.txt      |    2 +-
 drivers/ata/Kconfig                                |    8 +
 drivers/ata/Makefile                               |    1 +
 drivers/ata/sata_imx.c                             |  349 ++++++++++++++++++++
 4 files changed, 359 insertions(+), 1 deletions(-)
 create mode 100644 drivers/ata/sata_imx.c
Documentation/devicetree/bindings/ata/ahci-platform.txt +1 -1
index b519f9b..e252620 100644
@@ -4,7 +4,7 @@ SATA nodes are defined to describe on-chip Serial ATA controllers.
 Each SATA controller should have its own node.
 
 Required properties:
-- compatible        : compatible list, contains "calxeda,hb-ahci" or "snps,spear-ahci"
+- compatible        : compatible list, contains "calxeda,hb-ahci", "snps,spear-ahci" or "snps, imx-ahci"
 - interrupts        : <interrupt mapping for SATA IRQ>
 - reg               : <registers mapping>
 
drivers/ata/Kconfig +8 -0
index a5a3ebc..893fa0b 100644
@@ -236,6 +236,14 @@ config SATA_HIGHBANK
 
          If unsure, say N.
 
+config SATA_IMX
+       tristate "Freescale iMX AHCI SATA support"
+       help
+         This option enables support for the Freescale iMX SoC's
+         onboard AHCI SATA.
+
+         If unsure, say N.
+
 config SATA_MV
        tristate "Marvell SATA support"
        help
drivers/ata/Makefile +1 -0
index c04d0fd..c40b328 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_SATA_INIC162X)   += sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)       += sata_sil24.o
 obj-$(CONFIG_SATA_DWC)         += sata_dwc_460ex.o
 obj-$(CONFIG_SATA_HIGHBANK)    += sata_highbank.o libahci.o
+obj-$(CONFIG_SATA_IMX)         += sata_imx.o libahci.o
 
 # SFF w/ custom DMA
 obj-$(CONFIG_PDC_ADMA)         += pdc_adma.o
drivers/ata/sata_imx.c new file +349 -1
new file mode 100644
index 0000000..2be92e8
@@ -0,0 +1,349 @@
+/*
+ * Freescale IMX AHCI SATA platform driver
+ * Copyright 2013 Freescale Semiconductor, Inc.
+ *
+ * based on the AHCI SATA platform driver by Jeff Garzik and Anton Vorontsov
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/kernel.h>
+#include <linux/gfp.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/libata.h>
+#include <linux/ahci_platform.h>
+#include "ahci.h"
+
+enum {
+       HOST_TIMER1MS = 0xe0, /* Timer 1-ms */
+};
+
+static void ahci_imx_host_stop(struct ata_host *host);
+
+static struct ata_port_operations ahci_imx_platform_ops = {
+       .inherits       = &ahci_ops,
+       .host_stop      = ahci_imx_host_stop,
+};
+
+static const struct ata_port_info ahci_imx_port_info = {
+       .flags          = AHCI_FLAG_COMMON,
+       .pio_mask       = ATA_PIO4,
+       .udma_mask      = ATA_UDMA6,
+       .port_ops       = &ahci_imx_platform_ops,
+};
+
+static struct scsi_host_template ahci_imx_platform_sht = {
+       AHCI_SHT("sata_imx"),
+};
+
+/*
+ * Configure the HWINIT bits of the HOST_CAP and HOST_PORTS_IMPL,
+ * and IP vendor specific register HOST_TIMER1MS.
+ *
+ * Configure CAP_SSS (support stagered spin up).
+ * Implement the port0.
+ * Get the ahb clock rate, and configure the TIMER1MS register.
+ */
+static int imx_sata_init(void __iomem *mmio)
+{
+       int ret;
+       struct clk *ahb_clk;
+
+       ret = readl(mmio + HOST_CAP);
+       if (!(ret & HOST_CAP_SSS))
+               writel(ret |= HOST_CAP_SSS, mmio + HOST_CAP);
+       ret = readl(mmio + HOST_PORTS_IMPL);
+       if (!(ret & 0x1))
+               writel((ret | 0x1), mmio + HOST_PORTS_IMPL);
+       ahb_clk = clk_get_sys(NULL, "ahb");
+       if (IS_ERR(ahb_clk)) {
+               pr_err("no ahb clock.\n");
+               ret = PTR_ERR(ahb_clk);
+               return ret;
+       }
+       ret = clk_get_rate(ahb_clk) / 1000;
+       clk_put(ahb_clk);
+       writel(ret, mmio + HOST_TIMER1MS);
+
+       return ret;
+}
+
+static int ahci_imx_probe(struct platform_device *pdev)
+{
+       struct device *dev = &pdev->dev;
+       struct ahci_platform_data *pdata = dev_get_platdata(dev);
+       struct ata_port_info pi = ahci_imx_port_info;
+       const struct ata_port_info *ppi[] = { &pi, NULL };
+       struct ahci_host_priv *hpriv;
+       struct ata_host *host;
+       struct resource *mem;
+       int irq;
+       int n_ports;
+       int i;
+       int rc;
+
+       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       if (!mem) {
+               dev_err(dev, "no mmio space\n");
+               return -EINVAL;
+       }
+
+       irq = platform_get_irq(pdev, 0);
+       if (irq <= 0) {
+               dev_err(dev, "no irq\n");
+               return -EINVAL;
+       }
+
+       if (pdata && pdata->ata_port_info)
+               pi = *pdata->ata_port_info;
+
+       hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
+       if (!hpriv) {
+               dev_err(dev, "can't alloc ahci_host_priv\n");
+               return -ENOMEM;
+       }
+
+       hpriv->flags |= (unsigned long)pi.private_data;
+
+       hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
+       if (!hpriv->mmio) {
+               dev_err(dev, "can't map %pR\n", mem);
+               return -ENOMEM;
+       }
+
+       hpriv->clk = clk_get(dev, NULL);
+       if (IS_ERR(hpriv->clk)) {
+               dev_err(dev, "can't get clock\n");
+       } else {
+               rc = clk_prepare_enable(hpriv->clk);
+               if (rc) {
+                       dev_err(dev, "clock prepare enable failed");
+                       goto free_clk;
+               }
+       }
+
+       /*
+        * Some platforms might need to prepare for mmio region access,
+        * which could be done in the following init call. So, the mmio
+        * region shouldn't be accessed before init (if provided) has
+        * returned successfully.
+        */
+       if (pdata && pdata->init) {
+               rc = pdata->init(dev, hpriv->mmio);
+               if (rc)
+                       goto disable_unprepare_clk;
+       }
+
+       rc = imx_sata_init(hpriv->mmio);
+       if (rc < 0)
+               goto pdata_exit;
+
+       ahci_save_initial_config(dev, hpriv,
+               pdata ? pdata->force_port_map : 0,
+               pdata ? pdata->mask_port_map  : 0);
+
+       /* prepare host */
+       if (hpriv->cap & HOST_CAP_NCQ)
+               pi.flags |= ATA_FLAG_NCQ;
+
+       if (hpriv->cap & HOST_CAP_PMP)
+               pi.flags |= ATA_FLAG_PMP;
+
+       ahci_set_em_messages(hpriv, &pi);
+
+       /* CAP.NP sometimes indicate the index of the last enabled
+        * port, at other times, that of the last possible port, so
+        * determining the maximum port number requires looking at
+        * both CAP.NP and port_map.
+        */
+       n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
+
+       host = ata_host_alloc_pinfo(dev, ppi, n_ports);
+       if (!host) {
+               rc = -ENOMEM;
+               goto pdata_exit;
+       }
+
+       host->private_data = hpriv;
+
+       if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
+               host->flags |= ATA_HOST_PARALLEL_SCAN;
+       else
+               dev_info(dev, "ahci: SSS flag set, parallel bus scan disabled\n");
+
+       if (pi.flags & ATA_FLAG_EM)
+               ahci_reset_em(host);
+
+       for (i = 0; i < host->n_ports; i++) {
+               struct ata_port *ap = host->ports[i];
+
+               ata_port_desc(ap, "mmio %pR", mem);
+               ata_port_desc(ap, "port 0x%x", 0x100 + ap->port_no * 0x80);
+
+               /* set enclosure management message type */
+               if (ap->flags & ATA_FLAG_EM)
+                       ap->em_message_type = hpriv->em_msg_type;
+
+               /* disabled/not-implemented port */
+               if (!(hpriv->port_map & (1 << i)))
+                       ap->ops = &ata_dummy_port_ops;
+       }
+
+       rc = ahci_reset_controller(host);
+       if (rc)
+               goto pdata_exit;
+
+       ahci_init_controller(host);
+       ahci_print_info(host, "platform");
+
+       rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
+                              &ahci_imx_platform_sht);
+       if (rc)
+               goto pdata_exit;
+
+       return 0;
+pdata_exit:
+       if (pdata && pdata->exit)
+               pdata->exit(dev);
+disable_unprepare_clk:
+       if (!IS_ERR(hpriv->clk))
+               clk_disable_unprepare(hpriv->clk);
+free_clk:
+       if (!IS_ERR(hpriv->clk))
+               clk_put(hpriv->clk);
+       return rc;
+}
+
+static void ahci_imx_host_stop(struct ata_host *host)
+{
+       struct device *dev = host->dev;
+       struct ahci_platform_data *pdata = dev_get_platdata(dev);
+       struct ahci_host_priv *hpriv = host->private_data;
+
+       if (pdata && pdata->exit)
+               pdata->exit(dev);
+
+       if (!IS_ERR(hpriv->clk)) {
+               clk_disable_unprepare(hpriv->clk);
+               clk_put(hpriv->clk);
+       }
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ahci_imx_suspend(struct device *dev)
+{
+       struct ahci_platform_data *pdata = dev_get_platdata(dev);
+       struct ata_host *host = dev_get_drvdata(dev);
+       struct ahci_host_priv *hpriv = host->private_data;
+       void __iomem *mmio = hpriv->mmio;
+       u32 ctl;
+       int rc;
+
+       if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
+               dev_err(dev, "firmware update required for suspend/resume\n");
+               return -EIO;
+       }
+
+       /*
+        * AHCI spec rev1.1 section 8.3.3:
+        * Software must disable interrupts prior to requesting a
+        * transition of the HBA to D3 state.
+        */
+       ctl = readl(mmio + HOST_CTL);
+       ctl &= ~HOST_IRQ_EN;
+       writel(ctl, mmio + HOST_CTL);
+       readl(mmio + HOST_CTL); /* flush */
+
+       rc = ata_host_suspend(host, PMSG_SUSPEND);
+       if (rc)
+               return rc;
+
+       if (pdata && pdata->suspend)
+               return pdata->suspend(dev);
+
+       if (!IS_ERR(hpriv->clk))
+               clk_disable_unprepare(hpriv->clk);
+
+       return 0;
+}
+
+static int ahci_imx_resume(struct device *dev)
+{
+       struct ahci_platform_data *pdata = dev_get_platdata(dev);
+       struct ata_host *host = dev_get_drvdata(dev);
+       struct ahci_host_priv *hpriv = host->private_data;
+       int rc;
+
+       if (!IS_ERR(hpriv->clk)) {
+               rc = clk_prepare_enable(hpriv->clk);
+               if (rc) {
+                       dev_err(dev, "clock prepare enable failed");
+                       return rc;
+               }
+       }
+
+       if (pdata && pdata->resume) {
+               rc = pdata->resume(dev);
+               if (rc)
+                       goto disable_unprepare_clk;
+       }
+
+       if (dev->power.power_state.event == PM_EVENT_SUSPEND) {
+               rc = ahci_reset_controller(host);
+               if (rc)
+                       goto disable_unprepare_clk;
+
+               ahci_init_controller(host);
+       }
+
+       ata_host_resume(host);
+
+       return 0;
+
+disable_unprepare_clk:
+       if (!IS_ERR(hpriv->clk))
+               clk_disable_unprepare(hpriv->clk);
+
+       return rc;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(ahci_imx_pm_ops, ahci_imx_suspend, ahci_imx_resume);
+
+static const struct of_device_id ahci_of_match[] = {
+       { .compatible = "snps,imx-ahci", },
+       {},
+};
+MODULE_DEVICE_TABLE(of, ahci_of_match);
+
+static struct platform_driver ahci_imx_driver = {
+       .probe = ahci_imx_probe,
+       .remove = ata_platform_remove_one,
+       .driver = {
+               .name = "imx-ahci",
+               .owner = THIS_MODULE,
+               .of_match_table = ahci_of_match,
+               .pm = &ahci_imx_pm_ops,
+       },
+};
+module_platform_driver(ahci_imx_driver);
+
+MODULE_DESCRIPTION("FREESCALE IMX AHCI SATA platform driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("sata:imx");
--

1.7.5.4

--
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

#11 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

Quoted 5 lines · Girish K S

Hello Richard,

Instead of writing a separate driver for the changes you mentioned in the commit message. you can just add those changes to
the platform data (pdata-> init).

No. The platform init hook is not used to do IP block related setup. If we're mapping ahci block and setting up ahci registers in platform code, we are generally not doing the right thing.

Shawn

#12 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

Quoted 10 lines · Shawn Guo
Quoted 5 lines · Girish K S

Hello Richard,

Instead of writing a separate driver for the changes you mentioned in the commit message. you can just add those changes to
the platform data (pdata-> init).

No. The platform init hook is not used to do IP block related setup. If we're mapping ahci block and setting up ahci registers in platform code, we are generally not doing the right thing.

Yes I understand (anything specific to driver should be part of driver). I need to touch few platforms that are already doing this. Then how about keeping setup as part of driver data for the specific controller.

Quoted 3 lines

Shawn

#13 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

Quoted 8 lines · Girish K S
Quoted 7 lines · Shawn Guo
Quoted 5 lines · Girish K S

Hello Richard,

Instead of writing a separate driver for the changes you mentioned in the commit message. you can just add those changes to
the platform data (pdata-> init).

Quoted 7 lines
Quoted 3 lines

No. The platform init hook is not used to do IP block related setup. If we're mapping ahci block and setting up ahci registers in platform code, we are generally not doing the right thing.

Yes I understand (anything specific to driver should be part of driver). I need to touch few platforms that are already doing this. Then how about keeping setup as part of driver data for the specific controller.

I was too rush in writing the reply. Here is what I meant.

We should reuse the generic ahci_platform driver. And that means we have to use pdata->init() hook to do vendor specific setup. But the hook shouldn't be implemented in arch code (e.g. arch/arm/mach-imx/mach-imx6q.c), and it should be implemented in a vendor specific driver like sata_imx.c. IOW, we reuse everything implemented in the generic ahci_platform driver, and handle differences in vendor specific ahci driver via pdata->init() hook.

Does that make the most sense?

Shawn

maintainer
#14 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

In reply to Richard Zhu
Quoted 14 lines · Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers.
Because there are some misalignments of the bits definitions of the HBA registers and the Vendor Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock.

Quoted 4 lines

Setup its own ahci sata driver, enable the imx6q ahci sata support, and update the ahci sata binding document.

Wait a minute. We suggested to add a i.MX specific ahci driver to put the i.MX speicific setup in there. Now you really post a separate i.MX driver, but instead of putting the setup in there, it's a nearly identical copy of the generic driver *without* the setup and the setup is still in arch/arm/. That doesn't make sense to me.

drivers/ata/sata_imx.c line 74

Quoted 13 lines

+ */

drivers/ata/sata_imx.c new file +12 -0
+static int imx_sata_init(void __iomem *mmio)
+{
+	int ret;
+	struct clk *ahb_clk;
+
+	ret = readl(mmio + HOST_CAP);
+	if (!(ret & HOST_CAP_SSS))
+		writel(ret |= HOST_CAP_SSS, mmio + HOST_CAP);
+	ret = readl(mmio + HOST_PORTS_IMPL);
+	if (!(ret & 0x1))
+		writel((ret | 0x1), mmio + HOST_PORTS_IMPL);
+	ahb_clk = clk_get_sys(NULL, "ahb");

use devm_clk_get

Quoted 9 lines
drivers/ata/sata_imx.c new file +9 -0
+
+	if (pdata && pdata->ata_port_info)
+		pi = *pdata->ata_port_info;
+
+	hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
+	if (!hpriv) {
+		dev_err(dev, "can't alloc ahci_host_priv\n");
+		return -ENOMEM;
+	}

drivers/ata/sata_imx.c line 130

Quoted 10 lines
drivers/ata/sata_imx.c new file +10 -0
+
+	hpriv->flags |= (unsigned long)pi.private_data;
+
+	hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
+	if (!hpriv->mmio) {
+		dev_err(dev, "can't map %pR\n", mem);
+		return -ENOMEM;
+	}
+
+	hpriv->clk = clk_get(dev, NULL);

devm_clk_get.

drivers/ata/sata_imx.c line 132

Quoted 2 lines
drivers/ata/sata_imx.c new file +2 -0
+	if (IS_ERR(hpriv->clk)) {
+		dev_err(dev, "can't get clock\n");

That's an error. You should react to it.

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
#15 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

Quoted 18 lines · Sascha Hauer
Quoted 17 lines · Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers.
Because there are some misalignments of the bits definitions of the HBA registers and the Vendor Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock.

Setup its own ahci sata driver, enable the imx6q ahci sata support, and update the ahci sata binding document.

Quoted 7 lines
Quoted 1 line

Wait a minute. We suggested to add a i.MX specific ahci driver to put the i.MX speicific setup in there. Now you really post a separate i.MX driver, but instead of putting the setup in there, it's a nearly identical copy of the generic driver *without* the setup and the setup is still in arch/arm/. That doesn't make sense to me.

+1

Richard,

You could have confused by me with pointing you to the sata_highbank driver. Sorry. But Sascha posted his work [1] later in the same thread. That's obviously the way we should go, and I thought you knew you should start your v2 based on that. You missed that?

Shawn

[1] http://thread.gmane.org/gmane.linux.ide/54410/focus=54434

#16 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

Hi Shawn:
Never mind, I misunderstand what you said.

Now, I know what your are expecting now, would prepare the Version3, and send them later.

Best Regards
Richard Zhu

-----Original Message-----
From: Shawn Guo [mailto:shawn.guo at linaro.org]
Sent: Monday, July 01, 2013 11:30 PM
To: Sascha Hauer
Cc: Richard Zhu; linux-arm-kernel at lists.infradead.org; jgarzik at pobox.com; avorontsov at ru.mvista.com; rob.herring at calxeda.com; linux-ide at vger.kernel.org; Zhu Richard-R65037
Subject: Re: [v2 4/4] sata: imx: add ahci sata support on imx platforms

Quoted 15 lines · Sascha Hauer
Quoted 14 lines · Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers. Because there are some misalignments of the bits definitions of the HBA registers and the Vendor Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is
'0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL
should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should
be configured regarding to the frequency of AHB bus clock.

Setup its own ahci sata driver, enable the imx6q ahci sata support, and update the ahci sata binding document.

Quoted 7 lines
Quoted 1 line

Wait a minute. We suggested to add a i.MX specific ahci driver to put the i.MX speicific setup in there. Now you really post a separate i.MX driver, but instead of putting the setup in there, it's a nearly identical copy of the generic driver *without* the setup and the setup is still in arch/arm/. That doesn't make sense to me.

+1

Richard,

You could have confused by me with pointing you to the sata_highbank driver. Sorry. But Sascha posted his work [1] later in the same thread. That's obviously the way we should go, and I thought you knew you should start your v2 based on that. You missed that?

Shawn

[1] http://thread.gmane.org/gmane.linux.ide/54410/focus=54434

#17 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

In reply to Richard Zhu
Quoted 5 lines · Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers.
Because there are some misalignments of the bits definitions of the HBA registers and the Vendor

Quoted 8 lines

Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock.

All this really belongs in the bootloader. Wouldn't you most likely be booting the kernel from SATA as well? The first 2 are write once bits so setting them a 2nd time would have no effect.

I also agree that if this added, it should be added to the existing driver.

Rob

Quoted 413 lines

Setup its own ahci sata driver, enable the imx6q ahci sata support, and update the ahci sata binding document.

---
 .../devicetree/bindings/ata/ahci-platform.txt      |    2 +-
 drivers/ata/Kconfig                                |    8 +
 drivers/ata/Makefile                               |    1 +
 drivers/ata/sata_imx.c                             |  349 ++++++++++++++++++++
 4 files changed, 359 insertions(+), 1 deletions(-)
 create mode 100644 drivers/ata/sata_imx.c
Documentation/devicetree/bindings/ata/ahci-platform.txt +1 -1
index b519f9b..e252620 100644
@@ -4,7 +4,7 @@ SATA nodes are defined to describe on-chip Serial ATA controllers.
 Each SATA controller should have its own node.
 
 Required properties:
-- compatible        : compatible list, contains "calxeda,hb-ahci" or "snps,spear-ahci"
+- compatible        : compatible list, contains "calxeda,hb-ahci", "snps,spear-ahci" or "snps, imx-ahci"
 - interrupts        : <interrupt mapping for SATA IRQ>
 - reg               : <registers mapping>
 
drivers/ata/Kconfig +8 -0
index a5a3ebc..893fa0b 100644
@@ -236,6 +236,14 @@ config SATA_HIGHBANK
 
 	  If unsure, say N.
 
+config SATA_IMX
+	tristate "Freescale iMX AHCI SATA support"
+	help
+	  This option enables support for the Freescale iMX SoC's
+	  onboard AHCI SATA.
+
+	  If unsure, say N.
+
 config SATA_MV
 	tristate "Marvell SATA support"
 	help
drivers/ata/Makefile +1 -0
index c04d0fd..c40b328 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
 obj-$(CONFIG_SATA_DWC)		+= sata_dwc_460ex.o
 obj-$(CONFIG_SATA_HIGHBANK)	+= sata_highbank.o libahci.o
+obj-$(CONFIG_SATA_IMX)		+= sata_imx.o libahci.o
 
 # SFF w/ custom DMA
 obj-$(CONFIG_PDC_ADMA)		+= pdc_adma.o
drivers/ata/sata_imx.c new file +349 -0
new file mode 100644
index 0000000..2be92e8
@@ -0,0 +1,349 @@
+/*
+ * Freescale IMX AHCI SATA platform driver
+ * Copyright 2013 Freescale Semiconductor, Inc.
+ *
+ * based on the AHCI SATA platform driver by Jeff Garzik and Anton Vorontsov
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/kernel.h>
+#include <linux/gfp.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/libata.h>
+#include <linux/ahci_platform.h>
+#include "ahci.h"
+
+enum {
+	HOST_TIMER1MS = 0xe0, /* Timer 1-ms */
+};
+
+static void ahci_imx_host_stop(struct ata_host *host);
+
+static struct ata_port_operations ahci_imx_platform_ops = {
+	.inherits	= &ahci_ops,
+	.host_stop	= ahci_imx_host_stop,
+};
+
+static const struct ata_port_info ahci_imx_port_info = {
+	.flags		= AHCI_FLAG_COMMON,
+	.pio_mask	= ATA_PIO4,
+	.udma_mask	= ATA_UDMA6,
+	.port_ops	= &ahci_imx_platform_ops,
+};
+
+static struct scsi_host_template ahci_imx_platform_sht = {
+	AHCI_SHT("sata_imx"),
+};
+
+/*
+ * Configure the HWINIT bits of the HOST_CAP and HOST_PORTS_IMPL,
+ * and IP vendor specific register HOST_TIMER1MS.
+ *
+ * Configure CAP_SSS (support stagered spin up).
+ * Implement the port0.
+ * Get the ahb clock rate, and configure the TIMER1MS register.
+ */
+static int imx_sata_init(void __iomem *mmio)
+{
+	int ret;
+	struct clk *ahb_clk;
+
+	ret = readl(mmio + HOST_CAP);
+	if (!(ret & HOST_CAP_SSS))
+		writel(ret |= HOST_CAP_SSS, mmio + HOST_CAP);
+	ret = readl(mmio + HOST_PORTS_IMPL);
+	if (!(ret & 0x1))
+		writel((ret | 0x1), mmio + HOST_PORTS_IMPL);
+	ahb_clk = clk_get_sys(NULL, "ahb");
+	if (IS_ERR(ahb_clk)) {
+		pr_err("no ahb clock.\n");
+		ret = PTR_ERR(ahb_clk);
+		return ret;
+	}
+	ret = clk_get_rate(ahb_clk) / 1000;
+	clk_put(ahb_clk);
+	writel(ret, mmio + HOST_TIMER1MS);
+
+	return ret;
+}
+
+static int ahci_imx_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ahci_platform_data *pdata = dev_get_platdata(dev);
+	struct ata_port_info pi = ahci_imx_port_info;
+	const struct ata_port_info *ppi[] = { &pi, NULL };
+	struct ahci_host_priv *hpriv;
+	struct ata_host *host;
+	struct resource *mem;
+	int irq;
+	int n_ports;
+	int i;
+	int rc;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem) {
+		dev_err(dev, "no mmio space\n");
+		return -EINVAL;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq <= 0) {
+		dev_err(dev, "no irq\n");
+		return -EINVAL;
+	}
+
+	if (pdata && pdata->ata_port_info)
+		pi = *pdata->ata_port_info;
+
+	hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
+	if (!hpriv) {
+		dev_err(dev, "can't alloc ahci_host_priv\n");
+		return -ENOMEM;
+	}
+
+	hpriv->flags |= (unsigned long)pi.private_data;
+
+	hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
+	if (!hpriv->mmio) {
+		dev_err(dev, "can't map %pR\n", mem);
+		return -ENOMEM;
+	}
+
+	hpriv->clk = clk_get(dev, NULL);
+	if (IS_ERR(hpriv->clk)) {
+		dev_err(dev, "can't get clock\n");
+	} else {
+		rc = clk_prepare_enable(hpriv->clk);
+		if (rc) {
+			dev_err(dev, "clock prepare enable failed");
+			goto free_clk;
+		}
+	}
+
+	/*
+	 * Some platforms might need to prepare for mmio region access,
+	 * which could be done in the following init call. So, the mmio
+	 * region shouldn't be accessed before init (if provided) has
+	 * returned successfully.
+	 */
+	if (pdata && pdata->init) {
+		rc = pdata->init(dev, hpriv->mmio);
+		if (rc)
+			goto disable_unprepare_clk;
+	}
+
+	rc = imx_sata_init(hpriv->mmio);
+	if (rc < 0)
+		goto pdata_exit;
+
+	ahci_save_initial_config(dev, hpriv,
+		pdata ? pdata->force_port_map : 0,
+		pdata ? pdata->mask_port_map  : 0);
+
+	/* prepare host */
+	if (hpriv->cap & HOST_CAP_NCQ)
+		pi.flags |= ATA_FLAG_NCQ;
+
+	if (hpriv->cap & HOST_CAP_PMP)
+		pi.flags |= ATA_FLAG_PMP;
+
+	ahci_set_em_messages(hpriv, &pi);
+
+	/* CAP.NP sometimes indicate the index of the last enabled
+	 * port, at other times, that of the last possible port, so
+	 * determining the maximum port number requires looking at
+	 * both CAP.NP and port_map.
+	 */
+	n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
+
+	host = ata_host_alloc_pinfo(dev, ppi, n_ports);
+	if (!host) {
+		rc = -ENOMEM;
+		goto pdata_exit;
+	}
+
+	host->private_data = hpriv;
+
+	if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
+		host->flags |= ATA_HOST_PARALLEL_SCAN;
+	else
+		dev_info(dev, "ahci: SSS flag set, parallel bus scan disabled\n");
+
+	if (pi.flags & ATA_FLAG_EM)
+		ahci_reset_em(host);
+
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+
+		ata_port_desc(ap, "mmio %pR", mem);
+		ata_port_desc(ap, "port 0x%x", 0x100 + ap->port_no * 0x80);
+
+		/* set enclosure management message type */
+		if (ap->flags & ATA_FLAG_EM)
+			ap->em_message_type = hpriv->em_msg_type;
+
+		/* disabled/not-implemented port */
+		if (!(hpriv->port_map & (1 << i)))
+			ap->ops = &ata_dummy_port_ops;
+	}
+
+	rc = ahci_reset_controller(host);
+	if (rc)
+		goto pdata_exit;
+
+	ahci_init_controller(host);
+	ahci_print_info(host, "platform");
+
+	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
+			       &ahci_imx_platform_sht);
+	if (rc)
+		goto pdata_exit;
+
+	return 0;
+pdata_exit:
+	if (pdata && pdata->exit)
+		pdata->exit(dev);
+disable_unprepare_clk:
+	if (!IS_ERR(hpriv->clk))
+		clk_disable_unprepare(hpriv->clk);
+free_clk:
+	if (!IS_ERR(hpriv->clk))
+		clk_put(hpriv->clk);
+	return rc;
+}
+
+static void ahci_imx_host_stop(struct ata_host *host)
+{
+	struct device *dev = host->dev;
+	struct ahci_platform_data *pdata = dev_get_platdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+
+	if (pdata && pdata->exit)
+		pdata->exit(dev);
+
+	if (!IS_ERR(hpriv->clk)) {
+		clk_disable_unprepare(hpriv->clk);
+		clk_put(hpriv->clk);
+	}
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ahci_imx_suspend(struct device *dev)
+{
+	struct ahci_platform_data *pdata = dev_get_platdata(dev);
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	void __iomem *mmio = hpriv->mmio;
+	u32 ctl;
+	int rc;
+
+	if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
+		dev_err(dev, "firmware update required for suspend/resume\n");
+		return -EIO;
+	}
+
+	/*
+	 * AHCI spec rev1.1 section 8.3.3:
+	 * Software must disable interrupts prior to requesting a
+	 * transition of the HBA to D3 state.
+	 */
+	ctl = readl(mmio + HOST_CTL);
+	ctl &= ~HOST_IRQ_EN;
+	writel(ctl, mmio + HOST_CTL);
+	readl(mmio + HOST_CTL); /* flush */
+
+	rc = ata_host_suspend(host, PMSG_SUSPEND);
+	if (rc)
+		return rc;
+
+	if (pdata && pdata->suspend)
+		return pdata->suspend(dev);
+
+	if (!IS_ERR(hpriv->clk))
+		clk_disable_unprepare(hpriv->clk);
+
+	return 0;
+}
+
+static int ahci_imx_resume(struct device *dev)
+{
+	struct ahci_platform_data *pdata = dev_get_platdata(dev);
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	int rc;
+
+	if (!IS_ERR(hpriv->clk)) {
+		rc = clk_prepare_enable(hpriv->clk);
+		if (rc) {
+			dev_err(dev, "clock prepare enable failed");
+			return rc;
+		}
+	}
+
+	if (pdata && pdata->resume) {
+		rc = pdata->resume(dev);
+		if (rc)
+			goto disable_unprepare_clk;
+	}
+
+	if (dev->power.power_state.event == PM_EVENT_SUSPEND) {
+		rc = ahci_reset_controller(host);
+		if (rc)
+			goto disable_unprepare_clk;
+
+		ahci_init_controller(host);
+	}
+
+	ata_host_resume(host);
+
+	return 0;
+
+disable_unprepare_clk:
+	if (!IS_ERR(hpriv->clk))
+		clk_disable_unprepare(hpriv->clk);
+
+	return rc;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(ahci_imx_pm_ops, ahci_imx_suspend, ahci_imx_resume);
+
+static const struct of_device_id ahci_of_match[] = {
+	{ .compatible = "snps,imx-ahci", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ahci_of_match);
+
+static struct platform_driver ahci_imx_driver = {
+	.probe = ahci_imx_probe,
+	.remove = ata_platform_remove_one,
+	.driver = {
+		.name = "imx-ahci",
+		.owner = THIS_MODULE,
+		.of_match_table = ahci_of_match,
+		.pm = &ahci_imx_pm_ops,
+	},
+};
+module_platform_driver(ahci_imx_driver);
+
+MODULE_DESCRIPTION("FREESCALE IMX AHCI SATA platform driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("sata:imx");
maintainer
#18 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

Quoted 15 lines · Rob Herring
Quoted 5 lines · Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers.
Because there are some misalignments of the bits definitions of the HBA registers and the Vendor

Quoted 8 lines

Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock.

Quoted 1 line

All this really belongs in the bootloader.

I beg to differ. Normally we do not write the kernel that it depends on setup being done by the bootloader.

Quoted 2 lines

Wouldn't you most likely be
booting the kernel from SATA as well?

Having a SATA disk attached doesn't mean you boot from it.

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
#19 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

In reply to Rob Herring
Quoted 15 lines · Rob Herring
Quoted 5 lines · Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers.
Because there are some misalignments of the bits definitions of the HBA registers and the Vendor

Quoted 8 lines

Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock.

Quoted 3 lines

All this really belongs in the bootloader. Wouldn't you most likely be booting the kernel from SATA as well? The first 2 are write once bits so setting them a 2nd time would have no effect.

Even though its done in bootloader. It should be initialized in kernel. For a "suspend to disk" use case. If power to SATA controller is cut during suspend. Then this init sequence should be used

Quoted 423 lines

I also agree that if this added, it should be added to the existing driver.

Rob

Quoted 403 lines

Setup its own ahci sata driver, enable the imx6q ahci sata support, and update the ahci sata binding document.

---
 .../devicetree/bindings/ata/ahci-platform.txt      |    2 +-
 drivers/ata/Kconfig                                |    8 +
 drivers/ata/Makefile                               |    1 +
 drivers/ata/sata_imx.c                             |  349 ++++++++++++++++++++
 4 files changed, 359 insertions(+), 1 deletions(-)
 create mode 100644 drivers/ata/sata_imx.c
Documentation/devicetree/bindings/ata/ahci-platform.txt +1 -1
index b519f9b..e252620 100644
@@ -4,7 +4,7 @@ SATA nodes are defined to describe on-chip Serial ATA controllers.
 Each SATA controller should have its own node.
 
 Required properties:
-- compatible        : compatible list, contains "calxeda,hb-ahci" or "snps,spear-ahci"
+- compatible        : compatible list, contains "calxeda,hb-ahci", "snps,spear-ahci" or "snps, imx-ahci"
 - interrupts        : <interrupt mapping for SATA IRQ>
 - reg               : <registers mapping>
 
drivers/ata/Kconfig +8 -0
index a5a3ebc..893fa0b 100644
@@ -236,6 +236,14 @@ config SATA_HIGHBANK
 
        If unsure, say N.
 
+config SATA_IMX
+     tristate "Freescale iMX AHCI SATA support"
+     help
+       This option enables support for the Freescale iMX SoC's
+       onboard AHCI SATA.
+
+       If unsure, say N.
+
 config SATA_MV
      tristate "Marvell SATA support"
      help
drivers/ata/Makefile +1 -0
index c04d0fd..c40b328 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)     += sata_sil24.o
 obj-$(CONFIG_SATA_DWC)               += sata_dwc_460ex.o
 obj-$(CONFIG_SATA_HIGHBANK)  += sata_highbank.o libahci.o
+obj-$(CONFIG_SATA_IMX)               += sata_imx.o libahci.o
 
 # SFF w/ custom DMA
 obj-$(CONFIG_PDC_ADMA)               += pdc_adma.o
drivers/ata/sata_imx.c new file +339 -0
new file mode 100644
index 0000000..2be92e8
@@ -0,0 +1,349 @@
+/*
+ * Freescale IMX AHCI SATA platform driver
+ * Copyright 2013 Freescale Semiconductor, Inc.
+ *
+ * based on the AHCI SATA platform driver by Jeff Garzik and Anton Vorontsov
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/kernel.h>
+#include <linux/gfp.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/libata.h>
+#include <linux/ahci_platform.h>
+#include "ahci.h"
+
+enum {
+     HOST_TIMER1MS = 0xe0, /* Timer 1-ms */
+};
+
+static void ahci_imx_host_stop(struct ata_host *host);
+
+static struct ata_port_operations ahci_imx_platform_ops = {
+     .inherits       = &ahci_ops,
+     .host_stop      = ahci_imx_host_stop,
+};
+
+static const struct ata_port_info ahci_imx_port_info = {
+     .flags          = AHCI_FLAG_COMMON,
+     .pio_mask       = ATA_PIO4,
+     .udma_mask      = ATA_UDMA6,
+     .port_ops       = &ahci_imx_platform_ops,
+};
+
+static struct scsi_host_template ahci_imx_platform_sht = {
+     AHCI_SHT("sata_imx"),
+};
+
+/*
+ * Configure the HWINIT bits of the HOST_CAP and HOST_PORTS_IMPL,
+ * and IP vendor specific register HOST_TIMER1MS.
+ *
+ * Configure CAP_SSS (support stagered spin up).
+ * Implement the port0.
+ * Get the ahb clock rate, and configure the TIMER1MS register.
+ */
+static int imx_sata_init(void __iomem *mmio)
+{
+     int ret;
+     struct clk *ahb_clk;
+
+     ret = readl(mmio + HOST_CAP);
+     if (!(ret & HOST_CAP_SSS))
+             writel(ret |= HOST_CAP_SSS, mmio + HOST_CAP);
+     ret = readl(mmio + HOST_PORTS_IMPL);
+     if (!(ret & 0x1))
+             writel((ret | 0x1), mmio + HOST_PORTS_IMPL);
+     ahb_clk = clk_get_sys(NULL, "ahb");
+     if (IS_ERR(ahb_clk)) {
+             pr_err("no ahb clock.\n");
+             ret = PTR_ERR(ahb_clk);
+             return ret;
+     }
+     ret = clk_get_rate(ahb_clk) / 1000;
+     clk_put(ahb_clk);
+     writel(ret, mmio + HOST_TIMER1MS);
+
+     return ret;
+}
+
+static int ahci_imx_probe(struct platform_device *pdev)
+{
+     struct device *dev = &pdev->dev;
+     struct ahci_platform_data *pdata = dev_get_platdata(dev);
+     struct ata_port_info pi = ahci_imx_port_info;
+     const struct ata_port_info *ppi[] = { &pi, NULL };
+     struct ahci_host_priv *hpriv;
+     struct ata_host *host;
+     struct resource *mem;
+     int irq;
+     int n_ports;
+     int i;
+     int rc;
+
+     mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+     if (!mem) {
+             dev_err(dev, "no mmio space\n");
+             return -EINVAL;
+     }
+
+     irq = platform_get_irq(pdev, 0);
+     if (irq <= 0) {
+             dev_err(dev, "no irq\n");
+             return -EINVAL;
+     }
+
+     if (pdata && pdata->ata_port_info)
+             pi = *pdata->ata_port_info;
+
+     hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
+     if (!hpriv) {
+             dev_err(dev, "can't alloc ahci_host_priv\n");
+             return -ENOMEM;
+     }
+
+     hpriv->flags |= (unsigned long)pi.private_data;
+
+     hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
+     if (!hpriv->mmio) {
+             dev_err(dev, "can't map %pR\n", mem);
+             return -ENOMEM;
+     }
+
+     hpriv->clk = clk_get(dev, NULL);
+     if (IS_ERR(hpriv->clk)) {
+             dev_err(dev, "can't get clock\n");
+     } else {
+             rc = clk_prepare_enable(hpriv->clk);
+             if (rc) {
+                     dev_err(dev, "clock prepare enable failed");
+                     goto free_clk;
+             }
+     }
+
+     /*
+      * Some platforms might need to prepare for mmio region access,
+      * which could be done in the following init call. So, the mmio
+      * region shouldn't be accessed before init (if provided) has
+      * returned successfully.
+      */
+     if (pdata && pdata->init) {
+             rc = pdata->init(dev, hpriv->mmio);
+             if (rc)
+                     goto disable_unprepare_clk;
+     }
+
+     rc = imx_sata_init(hpriv->mmio);
+     if (rc < 0)
+             goto pdata_exit;
+
+     ahci_save_initial_config(dev, hpriv,
+             pdata ? pdata->force_port_map : 0,
+             pdata ? pdata->mask_port_map  : 0);
+
+     /* prepare host */
+     if (hpriv->cap & HOST_CAP_NCQ)
+             pi.flags |= ATA_FLAG_NCQ;
+
+     if (hpriv->cap & HOST_CAP_PMP)
+             pi.flags |= ATA_FLAG_PMP;
+
+     ahci_set_em_messages(hpriv, &pi);
+
+     /* CAP.NP sometimes indicate the index of the last enabled
+      * port, at other times, that of the last possible port, so
+      * determining the maximum port number requires looking at
+      * both CAP.NP and port_map.
+      */
+     n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
+
+     host = ata_host_alloc_pinfo(dev, ppi, n_ports);
+     if (!host) {
+             rc = -ENOMEM;
+             goto pdata_exit;
+     }
+
+     host->private_data = hpriv;
+
+     if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
+             host->flags |= ATA_HOST_PARALLEL_SCAN;
+     else
+             dev_info(dev, "ahci: SSS flag set, parallel bus scan disabled\n");
+
+     if (pi.flags & ATA_FLAG_EM)
+             ahci_reset_em(host);
+
+     for (i = 0; i < host->n_ports; i++) {
+             struct ata_port *ap = host->ports[i];
+
+             ata_port_desc(ap, "mmio %pR", mem);
+             ata_port_desc(ap, "port 0x%x", 0x100 + ap->port_no * 0x80);
+
+             /* set enclosure management message type */
+             if (ap->flags & ATA_FLAG_EM)
+                     ap->em_message_type = hpriv->em_msg_type;
+
+             /* disabled/not-implemented port */
+             if (!(hpriv->port_map & (1 << i)))
+                     ap->ops = &ata_dummy_port_ops;
+     }
+
+     rc = ahci_reset_controller(host);
+     if (rc)
+             goto pdata_exit;
+
+     ahci_init_controller(host);
+     ahci_print_info(host, "platform");
+
+     rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
+                            &ahci_imx_platform_sht);
+     if (rc)
+             goto pdata_exit;
+
+     return 0;
+pdata_exit:
+     if (pdata && pdata->exit)
+             pdata->exit(dev);
+disable_unprepare_clk:
+     if (!IS_ERR(hpriv->clk))
+             clk_disable_unprepare(hpriv->clk);
+free_clk:
+     if (!IS_ERR(hpriv->clk))
+             clk_put(hpriv->clk);
+     return rc;
+}
+
+static void ahci_imx_host_stop(struct ata_host *host)
+{
+     struct device *dev = host->dev;
+     struct ahci_platform_data *pdata = dev_get_platdata(dev);
+     struct ahci_host_priv *hpriv = host->private_data;
+
+     if (pdata && pdata->exit)
+             pdata->exit(dev);
+
+     if (!IS_ERR(hpriv->clk)) {
+             clk_disable_unprepare(hpriv->clk);
+             clk_put(hpriv->clk);
+     }
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ahci_imx_suspend(struct device *dev)
+{
+     struct ahci_platform_data *pdata = dev_get_platdata(dev);
+     struct ata_host *host = dev_get_drvdata(dev);
+     struct ahci_host_priv *hpriv = host->private_data;
+     void __iomem *mmio = hpriv->mmio;
+     u32 ctl;
+     int rc;
+
+     if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
+             dev_err(dev, "firmware update required for suspend/resume\n");
+             return -EIO;
+     }
+
+     /*
+      * AHCI spec rev1.1 section 8.3.3:
+      * Software must disable interrupts prior to requesting a
+      * transition of the HBA to D3 state.
+      */
+     ctl = readl(mmio + HOST_CTL);
+     ctl &= ~HOST_IRQ_EN;
+     writel(ctl, mmio + HOST_CTL);
+     readl(mmio + HOST_CTL); /* flush */
+
+     rc = ata_host_suspend(host, PMSG_SUSPEND);
+     if (rc)
+             return rc;
+
+     if (pdata && pdata->suspend)
+             return pdata->suspend(dev);
+
+     if (!IS_ERR(hpriv->clk))
+             clk_disable_unprepare(hpriv->clk);
+
+     return 0;
+}
+
+static int ahci_imx_resume(struct device *dev)
+{
+     struct ahci_platform_data *pdata = dev_get_platdata(dev);
+     struct ata_host *host = dev_get_drvdata(dev);
+     struct ahci_host_priv *hpriv = host->private_data;
+     int rc;
+
+     if (!IS_ERR(hpriv->clk)) {
+             rc = clk_prepare_enable(hpriv->clk);
+             if (rc) {
+                     dev_err(dev, "clock prepare enable failed");
+                     return rc;
+             }
+     }
+
+     if (pdata && pdata->resume) {
+             rc = pdata->resume(dev);
+             if (rc)
+                     goto disable_unprepare_clk;
+     }
+
+     if (dev->power.power_state.event == PM_EVENT_SUSPEND) {
+             rc = ahci_reset_controller(host);
+             if (rc)
+                     goto disable_unprepare_clk;
+
+             ahci_init_controller(host);
+     }
+
+     ata_host_resume(host);
+
+     return 0;
+
+disable_unprepare_clk:
+     if (!IS_ERR(hpriv->clk))
+             clk_disable_unprepare(hpriv->clk);
+
+     return rc;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(ahci_imx_pm_ops, ahci_imx_suspend, ahci_imx_resume);
+
+static const struct of_device_id ahci_of_match[] = {
+     { .compatible = "snps,imx-ahci", },
+     {},
+};
+MODULE_DEVICE_TABLE(of, ahci_of_match);
+
+static struct platform_driver ahci_imx_driver = {
+     .probe = ahci_imx_probe,
+     .remove = ata_platform_remove_one,
+     .driver = {
+             .name = "imx-ahci",

drivers/ata/sata_imx.c line 349

Quoted 10 lines
drivers/ata/sata_imx.c new file +10 -0
+             .owner = THIS_MODULE,
+             .of_match_table = ahci_of_match,
+             .pm = &ahci_imx_pm_ops,
+     },
+};
+module_platform_driver(ahci_imx_driver);
+
+MODULE_DESCRIPTION("FREESCALE IMX AHCI SATA platform driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("sata:imx");

--
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

#20 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

In reply to Richard Zhu
Quoted 5 lines · Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers.
Because there are some misalignments of the bits definitions of the HBA registers and the Vendor

Quoted 8 lines

Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock.

If these are the only differences, can't you make ahci_platform deal with different variants? That's how we deal with subtly different variants of about the same controllers.

Thanks.

Signature · 1 line
tejun
#21 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

Hi Tejun, Girish and all:
Yes, I understand what you said.
In the first version, based on the ahci_platform driver, I used to put the imx ahci differrence into
the platform level, and re-use ahci_platform driver.

Shawn has some concerns about that, just like he said.
So I re-orgnized imx ahci driver, and re-send it in the version2 around.

Let's have the discussions here, and figure out a most proper method to do the right thing. Any comments and suggetsions are apppreciated.
thanks again.

Best Regards
Richard Zhu
________________________________________
From: Tejun Heo [htejun at gmail.com] on behalf of Tejun Heo [tj at kernel.org]
Sent: Monday, July 01, 2013 9:36 PM
To: Richard Zhu
Cc: shawn.guo at linaro.org; linux-arm-kernel at lists.infradead.org; jgarzik at pobox.com; avorontsov at ru.mvista.com; rob.herring at calxeda.com; s.hauer at pengutronix.de; linux-ide at vger.kernel.org; Zhu Richard-R65037
Subject: Re: [v2 4/4] sata: imx: add ahci sata support on imx platforms

Quoted 5 lines · Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers.
Because there are some misalignments of the bits definitions of the HBA registers and the Vendor

Quoted 8 lines

Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock.

If these are the only differences, can't you make ahci_platform deal with different variants? That's how we deal with subtly different variants of about the same controllers.

Thanks.

--
tejun

#22 lore raw

PATCH v2 4/4 sata: imx: add ahci sata support on imx platforms

In reply to Tejun Heo
Quoted 15 lines · Tejun Heo
Quoted 5 lines · Richard Zhu

imx6q contains one Synopsys AHCI SATA controller, But it can't shares ahci_platform driver with other controllers.
Because there are some misalignments of the bits definitions of the HBA registers and the Vendor

Quoted 8 lines

Specific registers
- CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1'
- bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0)
- One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock.

Quoted 3 lines

If these are the only differences, can't you make ahci_platform deal with different variants? That's how we deal with subtly different variants of about the same controllers.

There are more:

- There is an additional phy clock to manage
- There are some i.MX SoC integration level registers to set up

I prefer to that we reuse the generic ahci_platform stuff, and handle the differences in a sata_imx.c via pdata->init() interface.

Shawn