[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

Cc: devicetree@vger.kernel.org
Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 Documentation/devicetree/bindings/ata/ahci-st.txt | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt

diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt
new file mode 100644
index 0000000..1b69fa9
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-st.txt
@@ -0,0 +1,31 @@
+STMicroelectronics STi SATA controller
+
+This binding describes a SATA device.
+
+Required properties:
+ - compatible	   : Must be "st,ahci"
+ - reg		   : Physical base addresses and length of register sets
+ - interrupts	   : Interrupt associated with the SATA device
+ - interrupt-names :   Associated name must be; "hostc"
+ - resets	   : The power-down and soft-reset lines of SATA IP
+ - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"
+ - clocks	   : The phandle for the clock
+ - clock-names	   :   Associated name must be; "ahci_clk"
+ - phys		   : The phandle for the PHY device
+ - phy-names	   :   Associated name must be; "ahci_phy"
+
+Example:
+
+	sata0: sata@fe380000 {
+		compatible      = "st,ahci";
+		reg             = <0xfe380000 0x1000>;
+		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
+		interrupt-names = "hostc";
+		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+		phy-names       = "ahci_phy";
+		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
+				  <&softreset STIH416_SATA0_SOFTRESET>;
+		reset-names     = "pwr-dwn", "sw-rst";
+		clocks	        = <&CLK_S_ICN_REG_0>;
+		clock-names     = "ahci_clk";
+	};
-- 
1.8.3.2

[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
Acked-by: Alexandre Torgue <alexandre.torgue-qxv4g6HH51o@public.gmane.org>
Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/ata/ahci-st.txt | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt

diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt
new file mode 100644
index 0000000..1b69fa9
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-st.txt
@@ -0,0 +1,31 @@
+STMicroelectronics STi SATA controller
+
+This binding describes a SATA device.
+
+Required properties:
+ - compatible	   : Must be "st,ahci"
+ - reg		   : Physical base addresses and length of register sets
+ - interrupts	   : Interrupt associated with the SATA device
+ - interrupt-names :   Associated name must be; "hostc"
+ - resets	   : The power-down and soft-reset lines of SATA IP
+ - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"
+ - clocks	   : The phandle for the clock
+ - clock-names	   :   Associated name must be; "ahci_clk"
+ - phys		   : The phandle for the PHY device
+ - phy-names	   :   Associated name must be; "ahci_phy"
+
+Example:
+
+	sata0: sata@fe380000 {
+		compatible      = "st,ahci";
+		reg             = <0xfe380000 0x1000>;
+		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
+		interrupt-names = "hostc";
+		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+		phy-names       = "ahci_phy";
+		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
+				  <&softreset STIH416_SATA0_SOFTRESET>;
+		reset-names     = "pwr-dwn", "sw-rst";
+		clocks	        = <&CLK_S_ICN_REG_0>;
+		clock-names     = "ahci_clk";
+	};
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

Cc: devicetree at vger.kernel.org
Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 Documentation/devicetree/bindings/ata/ahci-st.txt | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt

diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt
new file mode 100644
index 0000000..1b69fa9
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-st.txt
@@ -0,0 +1,31 @@
+STMicroelectronics STi SATA controller
+
+This binding describes a SATA device.
+
+Required properties:
+ - compatible	   : Must be "st,ahci"
+ - reg		   : Physical base addresses and length of register sets
+ - interrupts	   : Interrupt associated with the SATA device
+ - interrupt-names :   Associated name must be; "hostc"
+ - resets	   : The power-down and soft-reset lines of SATA IP
+ - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"
+ - clocks	   : The phandle for the clock
+ - clock-names	   :   Associated name must be; "ahci_clk"
+ - phys		   : The phandle for the PHY device
+ - phy-names	   :   Associated name must be; "ahci_phy"
+
+Example:
+
+	sata0: sata at fe380000 {
+		compatible      = "st,ahci";
+		reg             = <0xfe380000 0x1000>;
+		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
+		interrupt-names = "hostc";
+		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+		phy-names       = "ahci_phy";
+		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
+				  <&softreset STIH416_SATA0_SOFTRESET>;
+		reset-names     = "pwr-dwn", "sw-rst";
+		clocks	        = <&CLK_S_ICN_REG_0>;
+		clock-names     = "ahci_clk";
+	};
-- 
1.8.3.2

[PATCH v2 2/3] ARM: DT: STi: Add DT node for ST's SATA device

[Lee Jones]

Cc: devicetree@vger.kernel.org
Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stih416-b2020-revE.dts |  4 ++++
 arch/arm/boot/dts/stih416-b2020.dts      |  4 ++++
 arch/arm/boot/dts/stih416.dtsi           | 16 ++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/stih416-b2020-revE.dts b/arch/arm/boot/dts/stih416-b2020-revE.dts
index 693d0ec..7350a86 100644
--- a/arch/arm/boot/dts/stih416-b2020-revE.dts
+++ b/arch/arm/boot/dts/stih416-b2020-revE.dts
@@ -37,5 +37,9 @@
 			st,pcie-tx-pol-inv;
 			st,sata-gen = <3>;
 		};
+
+		sata0: sata@fe380000{
+			status = "okay";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/stih416-b2020.dts b/arch/arm/boot/dts/stih416-b2020.dts
index fd9cbad..ebd784b 100644
--- a/arch/arm/boot/dts/stih416-b2020.dts
+++ b/arch/arm/boot/dts/stih416-b2020.dts
@@ -18,5 +18,9 @@
 			st,pcie_tx_pol_inv = <1>;
 			st,sata_gen = "gen3";
 		};
+
+		sata0: sata@fe380000{
+			status = "okay";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index 0d3f59c..313d4fd 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -205,5 +205,21 @@
 			#phy-cells 	= <2>;
 			st,syscfg  	= <&syscfg_rear>;
 		};
+
+		sata0: sata@fe380000 {
+			compatible      = "st,ahci";
+			reg             = <0xfe380000 0x1000>;
+			interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
+			interrupt-names = "hostc";
+			phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+			phy-names       = "sata-phy";
+			resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
+					  <&softreset STIH416_SATA0_SOFTRESET>;
+			reset-names     = "pwr-dwn", "sw-rst";
+			clock-names     = "ahci_clk";
+			clocks	        = <&CLK_S_ICN_REG_0>;
+
+			status	        = "disabled";
+		};
 	};
 };
-- 
1.8.3.2

[PATCH v2 2/3] ARM: DT: STi: Add DT node for ST's SATA device

[Lee Jones]

Cc: devicetree at vger.kernel.org
Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stih416-b2020-revE.dts |  4 ++++
 arch/arm/boot/dts/stih416-b2020.dts      |  4 ++++
 arch/arm/boot/dts/stih416.dtsi           | 16 ++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/stih416-b2020-revE.dts b/arch/arm/boot/dts/stih416-b2020-revE.dts
index 693d0ec..7350a86 100644
--- a/arch/arm/boot/dts/stih416-b2020-revE.dts
+++ b/arch/arm/boot/dts/stih416-b2020-revE.dts
@@ -37,5 +37,9 @@
 			st,pcie-tx-pol-inv;
 			st,sata-gen = <3>;
 		};
+
+		sata0: sata at fe380000{
+			status = "okay";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/stih416-b2020.dts b/arch/arm/boot/dts/stih416-b2020.dts
index fd9cbad..ebd784b 100644
--- a/arch/arm/boot/dts/stih416-b2020.dts
+++ b/arch/arm/boot/dts/stih416-b2020.dts
@@ -18,5 +18,9 @@
 			st,pcie_tx_pol_inv = <1>;
 			st,sata_gen = "gen3";
 		};
+
+		sata0: sata at fe380000{
+			status = "okay";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index 0d3f59c..313d4fd 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -205,5 +205,21 @@
 			#phy-cells 	= <2>;
 			st,syscfg  	= <&syscfg_rear>;
 		};
+
+		sata0: sata at fe380000 {
+			compatible      = "st,ahci";
+			reg             = <0xfe380000 0x1000>;
+			interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
+			interrupt-names = "hostc";
+			phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+			phy-names       = "sata-phy";
+			resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
+					  <&softreset STIH416_SATA0_SOFTRESET>;
+			reset-names     = "pwr-dwn", "sw-rst";
+			clock-names     = "ahci_clk";
+			clocks	        = <&CLK_S_ICN_REG_0>;
+
+			status	        = "disabled";
+		};
 	};
 };
-- 
1.8.3.2

[PATCH v2 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

Cc: Tejun Heo <tj@kernel.org>
Cc: linux-ide@vger.kernel.org
Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/ata/Kconfig   |   9 ++
 drivers/ata/Makefile  |   1 +
 drivers/ata/ahci_st.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 260 insertions(+)
 create mode 100644 drivers/ata/ahci_st.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 96176f4..968ca2f 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -98,6 +98,15 @@ config SATA_AHCI_PLATFORM
 
 	  If unsure, say N.
 
+config SATA_AHCI_ST
+	tristate "ST SATA support"
+	depends on SATA_AHCI_PLATFORM
+	select GENERIC_PHY
+	help
+	  This option enables support for ST SATA controller.
+
+	  If unsure, say N.
+
 config AHCI_IMX
 	tristate "Freescale i.MX AHCI SATA support"
 	depends on SATA_AHCI_PLATFORM && MFD_SYSCON
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 246050b..6bbd6da 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_ATA)		+= libata.o
 obj-$(CONFIG_SATA_AHCI)		+= ahci.o libahci.o
 obj-$(CONFIG_SATA_ACARD_AHCI)	+= acard-ahci.o libahci.o
 obj-$(CONFIG_SATA_AHCI_PLATFORM) += ahci_platform.o libahci.o
+obj-$(CONFIG_SATA_AHCI_ST)      += ahci_st.o
 obj-$(CONFIG_SATA_FSL)		+= sata_fsl.o
 obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
new file mode 100644
index 0000000..91887a0
--- /dev/null
+++ b/drivers/ata/ahci_st.c
@@ -0,0 +1,250 @@
+/*
+ * Copyright (C) 2012 STMicroelectronics Limited
+ *
+ * Authors: Francesco Virlinzi <francesco.virlinzi@st.com>
+ *	    Alexandre Torgue <alexandre.torgue@st.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/export.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/ahci_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/libata.h>
+#include <linux/reset.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+
+#include "ahci.h"
+
+#define ST_AHCI_OOBR			0xbc
+#define ST_AHCI_OOBR_WE			BIT(31)
+#define ST_AHCI_OOBR_CWMIN_SHIFT	24
+#define ST_AHCI_OOBR_CWMAX_SHIFT	16
+#define ST_AHCI_OOBR_CIMIN_SHIFT	8
+#define ST_AHCI_OOBR_CIMAX_SHIFT	0
+
+struct st_ahci_drv_data {
+	struct platform_device *ahci;
+	struct phy *phy;
+	struct reset_control *pwr;
+	struct reset_control *sw_rst;
+	struct reset_control *pwr_rst;
+	struct ahci_host_priv *hpriv;
+};
+
+static void st_ahci_configure_oob(void __iomem *mmio)
+{
+	unsigned long old_val, new_val;
+
+	new_val = (0x02 << ST_AHCI_OOBR_CWMIN_SHIFT) |
+		  (0x04 << ST_AHCI_OOBR_CWMAX_SHIFT) |
+		  (0x08 << ST_AHCI_OOBR_CIMIN_SHIFT) |
+		  (0x0C << ST_AHCI_OOBR_CIMAX_SHIFT);
+
+	old_val = readl(mmio + ST_AHCI_OOBR);
+	writel(old_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
+	writel(new_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
+	writel(new_val, mmio + ST_AHCI_OOBR);
+}
+
+static int st_ahci_deassert_resets(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_deassert(drv_data->pwr);
+		if (err) {
+			dev_err(dev, "unable to bring out of pwrdwn\n");
+			return err;
+		}
+	}
+
+	st_ahci_configure_oob(drv_data->hpriv->mmio);
+
+	if (drv_data->sw_rst) {
+		err = reset_control_deassert(drv_data->sw_rst);
+		if (err) {
+			dev_err(dev, "unable to bring out of sw-rst\n");
+			return err;
+		}
+	}
+
+	if (drv_data->pwr_rst) {
+		err = reset_control_deassert(drv_data->pwr_rst);
+		if (err) {
+			dev_err(dev, "unable to bring out of pwr-rst\n");
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int st_ahci_probe_resets(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+
+	drv_data->pwr = devm_reset_control_get(&pdev->dev, "pwr-dwn");
+	if (IS_ERR(drv_data->pwr)) {
+		dev_info(&pdev->dev, "power reset control not defined\n");
+		drv_data->pwr = NULL;
+	}
+
+	drv_data->sw_rst = devm_reset_control_get(&pdev->dev, "sw-rst");
+	if (IS_ERR(drv_data->sw_rst)) {
+		dev_info(&pdev->dev, "soft reset control not defined\n");
+		drv_data->sw_rst = NULL;
+	}
+
+	drv_data->pwr_rst = devm_reset_control_get(&pdev->dev, "pwr-rst");
+	if (IS_ERR(drv_data->pwr_rst)) {
+		dev_dbg(&pdev->dev, "power soft reset control not defined\n");
+		drv_data->pwr_rst = NULL;
+	}
+
+	return st_ahci_deassert_resets(&pdev->dev);
+}
+
+static const struct ata_port_info st_ahci_port_info = {
+	.flags          = AHCI_FLAG_COMMON,
+	.pio_mask       = ATA_PIO4,
+	.udma_mask      = ATA_UDMA6,
+	.port_ops       = &ahci_platform_ops,
+};
+
+static int st_ahci_probe(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data;
+	struct ahci_host_priv *hpriv;
+	int err;
+
+	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
+	if (!drv_data)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, drv_data);
+
+	hpriv = ahci_platform_get_resources(pdev);
+	if (IS_ERR(hpriv))
+		return PTR_ERR(hpriv);
+
+	drv_data->hpriv = hpriv;
+
+	err = st_ahci_probe_resets(pdev);
+	if (err)
+		goto put_resources;
+
+	err = ahci_platform_enable_resources(hpriv);
+	if (err)
+		goto put_resources;
+
+	err = ahci_platform_init_host(pdev, hpriv, &st_ahci_port_info, 0, 0);
+	if (err)
+		goto disable_resources;
+
+	return 0;
+
+disable_resources:
+       ahci_platform_disable_resources(hpriv);
+put_resources:
+       ahci_platform_put_resources(&pdev->dev, hpriv);
+
+       return err;
+}
+
+static int st_ahci_remove(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_assert(drv_data->pwr);
+		if (err)
+			dev_err(&pdev->dev, "unable to pwrdwn\n");
+	}
+
+	ahci_platform_disable_resources(hpriv);
+	ahci_platform_put_resources(&pdev->dev, hpriv);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int st_ahci_suspend(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_assert(drv_data->pwr);
+		if (err) {
+			dev_err(dev, "unable to pwrdwn");
+			return err;
+		}
+	}
+
+	ahci_platform_disable_resources(hpriv);
+
+	return 0;
+}
+
+static int st_ahci_resume(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	err = ahci_platform_enable_resources(hpriv);
+	if (err)
+		goto put_resources;
+
+	err = st_ahci_deassert_resets(dev);
+	if (err)
+		goto disable_resources;
+
+	return 0;
+
+disable_resources:
+       ahci_platform_disable_resources(hpriv);
+put_resources:
+       ahci_platform_put_resources(dev, hpriv);
+
+       return err;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(st_ahci_pm_ops, st_ahci_suspend, st_ahci_resume);
+
+static struct of_device_id st_ahci_match[] = {
+	{ .compatible = "st,ahci", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, st_ahci_match);
+
+static struct platform_driver st_ahci_driver = {
+	.driver = {
+		.name = "st_ahci",
+		.owner = THIS_MODULE,
+		.pm = &st_ahci_pm_ops,
+		.of_match_table = of_match_ptr(st_ahci_match),
+	},
+	.probe = st_ahci_probe,
+	.remove = st_ahci_remove,
+};
+module_platform_driver(st_ahci_driver);
+
+MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@st.com>");
+MODULE_AUTHOR("Francesco Virlinzi <francesco.virlinzi@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics Sata Ahci driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.2

[PATCH v2 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

Cc: Tejun Heo <tj@kernel.org>
Cc: linux-ide at vger.kernel.org
Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/ata/Kconfig   |   9 ++
 drivers/ata/Makefile  |   1 +
 drivers/ata/ahci_st.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 260 insertions(+)
 create mode 100644 drivers/ata/ahci_st.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 96176f4..968ca2f 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -98,6 +98,15 @@ config SATA_AHCI_PLATFORM
 
 	  If unsure, say N.
 
+config SATA_AHCI_ST
+	tristate "ST SATA support"
+	depends on SATA_AHCI_PLATFORM
+	select GENERIC_PHY
+	help
+	  This option enables support for ST SATA controller.
+
+	  If unsure, say N.
+
 config AHCI_IMX
 	tristate "Freescale i.MX AHCI SATA support"
 	depends on SATA_AHCI_PLATFORM && MFD_SYSCON
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 246050b..6bbd6da 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_ATA)		+= libata.o
 obj-$(CONFIG_SATA_AHCI)		+= ahci.o libahci.o
 obj-$(CONFIG_SATA_ACARD_AHCI)	+= acard-ahci.o libahci.o
 obj-$(CONFIG_SATA_AHCI_PLATFORM) += ahci_platform.o libahci.o
+obj-$(CONFIG_SATA_AHCI_ST)      += ahci_st.o
 obj-$(CONFIG_SATA_FSL)		+= sata_fsl.o
 obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
new file mode 100644
index 0000000..91887a0
--- /dev/null
+++ b/drivers/ata/ahci_st.c
@@ -0,0 +1,250 @@
+/*
+ * Copyright (C) 2012 STMicroelectronics Limited
+ *
+ * Authors: Francesco Virlinzi <francesco.virlinzi@st.com>
+ *	    Alexandre Torgue <alexandre.torgue@st.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/export.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/ahci_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/libata.h>
+#include <linux/reset.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+
+#include "ahci.h"
+
+#define ST_AHCI_OOBR			0xbc
+#define ST_AHCI_OOBR_WE			BIT(31)
+#define ST_AHCI_OOBR_CWMIN_SHIFT	24
+#define ST_AHCI_OOBR_CWMAX_SHIFT	16
+#define ST_AHCI_OOBR_CIMIN_SHIFT	8
+#define ST_AHCI_OOBR_CIMAX_SHIFT	0
+
+struct st_ahci_drv_data {
+	struct platform_device *ahci;
+	struct phy *phy;
+	struct reset_control *pwr;
+	struct reset_control *sw_rst;
+	struct reset_control *pwr_rst;
+	struct ahci_host_priv *hpriv;
+};
+
+static void st_ahci_configure_oob(void __iomem *mmio)
+{
+	unsigned long old_val, new_val;
+
+	new_val = (0x02 << ST_AHCI_OOBR_CWMIN_SHIFT) |
+		  (0x04 << ST_AHCI_OOBR_CWMAX_SHIFT) |
+		  (0x08 << ST_AHCI_OOBR_CIMIN_SHIFT) |
+		  (0x0C << ST_AHCI_OOBR_CIMAX_SHIFT);
+
+	old_val = readl(mmio + ST_AHCI_OOBR);
+	writel(old_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
+	writel(new_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
+	writel(new_val, mmio + ST_AHCI_OOBR);
+}
+
+static int st_ahci_deassert_resets(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_deassert(drv_data->pwr);
+		if (err) {
+			dev_err(dev, "unable to bring out of pwrdwn\n");
+			return err;
+		}
+	}
+
+	st_ahci_configure_oob(drv_data->hpriv->mmio);
+
+	if (drv_data->sw_rst) {
+		err = reset_control_deassert(drv_data->sw_rst);
+		if (err) {
+			dev_err(dev, "unable to bring out of sw-rst\n");
+			return err;
+		}
+	}
+
+	if (drv_data->pwr_rst) {
+		err = reset_control_deassert(drv_data->pwr_rst);
+		if (err) {
+			dev_err(dev, "unable to bring out of pwr-rst\n");
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int st_ahci_probe_resets(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+
+	drv_data->pwr = devm_reset_control_get(&pdev->dev, "pwr-dwn");
+	if (IS_ERR(drv_data->pwr)) {
+		dev_info(&pdev->dev, "power reset control not defined\n");
+		drv_data->pwr = NULL;
+	}
+
+	drv_data->sw_rst = devm_reset_control_get(&pdev->dev, "sw-rst");
+	if (IS_ERR(drv_data->sw_rst)) {
+		dev_info(&pdev->dev, "soft reset control not defined\n");
+		drv_data->sw_rst = NULL;
+	}
+
+	drv_data->pwr_rst = devm_reset_control_get(&pdev->dev, "pwr-rst");
+	if (IS_ERR(drv_data->pwr_rst)) {
+		dev_dbg(&pdev->dev, "power soft reset control not defined\n");
+		drv_data->pwr_rst = NULL;
+	}
+
+	return st_ahci_deassert_resets(&pdev->dev);
+}
+
+static const struct ata_port_info st_ahci_port_info = {
+	.flags          = AHCI_FLAG_COMMON,
+	.pio_mask       = ATA_PIO4,
+	.udma_mask      = ATA_UDMA6,
+	.port_ops       = &ahci_platform_ops,
+};
+
+static int st_ahci_probe(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data;
+	struct ahci_host_priv *hpriv;
+	int err;
+
+	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
+	if (!drv_data)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, drv_data);
+
+	hpriv = ahci_platform_get_resources(pdev);
+	if (IS_ERR(hpriv))
+		return PTR_ERR(hpriv);
+
+	drv_data->hpriv = hpriv;
+
+	err = st_ahci_probe_resets(pdev);
+	if (err)
+		goto put_resources;
+
+	err = ahci_platform_enable_resources(hpriv);
+	if (err)
+		goto put_resources;
+
+	err = ahci_platform_init_host(pdev, hpriv, &st_ahci_port_info, 0, 0);
+	if (err)
+		goto disable_resources;
+
+	return 0;
+
+disable_resources:
+       ahci_platform_disable_resources(hpriv);
+put_resources:
+       ahci_platform_put_resources(&pdev->dev, hpriv);
+
+       return err;
+}
+
+static int st_ahci_remove(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_assert(drv_data->pwr);
+		if (err)
+			dev_err(&pdev->dev, "unable to pwrdwn\n");
+	}
+
+	ahci_platform_disable_resources(hpriv);
+	ahci_platform_put_resources(&pdev->dev, hpriv);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int st_ahci_suspend(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_assert(drv_data->pwr);
+		if (err) {
+			dev_err(dev, "unable to pwrdwn");
+			return err;
+		}
+	}
+
+	ahci_platform_disable_resources(hpriv);
+
+	return 0;
+}
+
+static int st_ahci_resume(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	err = ahci_platform_enable_resources(hpriv);
+	if (err)
+		goto put_resources;
+
+	err = st_ahci_deassert_resets(dev);
+	if (err)
+		goto disable_resources;
+
+	return 0;
+
+disable_resources:
+       ahci_platform_disable_resources(hpriv);
+put_resources:
+       ahci_platform_put_resources(dev, hpriv);
+
+       return err;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(st_ahci_pm_ops, st_ahci_suspend, st_ahci_resume);
+
+static struct of_device_id st_ahci_match[] = {
+	{ .compatible = "st,ahci", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, st_ahci_match);
+
+static struct platform_driver st_ahci_driver = {
+	.driver = {
+		.name = "st_ahci",
+		.owner = THIS_MODULE,
+		.pm = &st_ahci_pm_ops,
+		.of_match_table = of_match_ptr(st_ahci_match),
+	},
+	.probe = st_ahci_probe,
+	.remove = st_ahci_remove,
+};
+module_platform_driver(st_ahci_driver);
+
+MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@st.com>");
+MODULE_AUTHOR("Francesco Virlinzi <francesco.virlinzi@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics Sata Ahci driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.2

Re: [PATCH v2 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

On Tue, Feb 25, 2014 at 11:41:21AM +0000, Lee Jones wrote:
> Cc: Tejun Heo <tj@kernel.org>
> Cc: linux-ide@vger.kernel.org
> Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Applied to libata/for-3.15 w/ missing "\n" added to dev_err() in
st_ahci_suspend().

Thanks.

-- 
tejun

[PATCH v2 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

On Tue, Feb 25, 2014 at 11:41:21AM +0000, Lee Jones wrote:
> Cc: Tejun Heo <tj@kernel.org>
> Cc: linux-ide at vger.kernel.org
> Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Applied to libata/for-3.15 w/ missing "\n" added to dev_err() in
st_ahci_suspend().

Thanks.

-- 
tejun

Re: [PATCH v2 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

On Tue, Feb 25, 2014 at 09:41:41AM -0500, Tejun Heo wrote:
> On Tue, Feb 25, 2014 at 11:41:21AM +0000, Lee Jones wrote:
> > Cc: Tejun Heo <tj@kernel.org>
> > Cc: linux-ide@vger.kernel.org
> > Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Applied to libata/for-3.15 w/ missing "\n" added to dev_err() in
> st_ahci_suspend().

Reverted from libata/for-3.15 due to build failure.  Please drop
ahci_platform_put_resource() calls.  ahci_platform now manages it
through devres.

-- 
tejun

[PATCH v2 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

On Tue, Feb 25, 2014 at 09:41:41AM -0500, Tejun Heo wrote:
> On Tue, Feb 25, 2014 at 11:41:21AM +0000, Lee Jones wrote:
> > Cc: Tejun Heo <tj@kernel.org>
> > Cc: linux-ide at vger.kernel.org
> > Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Applied to libata/for-3.15 w/ missing "\n" added to dev_err() in
> st_ahci_suspend().

Reverted from libata/for-3.15 due to build failure.  Please drop
ahci_platform_put_resource() calls.  ahci_platform now manages it
through devres.

-- 
tejun

Re: [PATCH v2 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

> > > Cc: Tejun Heo <tj@kernel.org>
> > > Cc: linux-ide@vger.kernel.org
> > > Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > Applied to libata/for-3.15 w/ missing "\n" added to dev_err() in
> > st_ahci_suspend().
> 
> Reverted from libata/for-3.15 due to build failure.  Please drop
> ahci_platform_put_resource() calls.  ahci_platform now manages it
> through devres.

Ah, nice fixup.

I'll rebase onto your most recent tree and retest.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v2 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

> > > Cc: Tejun Heo <tj@kernel.org>
> > > Cc: linux-ide at vger.kernel.org
> > > Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > Applied to libata/for-3.15 w/ missing "\n" added to dev_err() in
> > st_ahci_suspend().
> 
> Reverted from libata/for-3.15 due to build failure.  Please drop
> ahci_platform_put_resource() calls.  ahci_platform now manages it
> through devres.

Ah, nice fixup.

I'll rebase onto your most recent tree and retest.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

ahci: st: Add support for ST's SATA IP

Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index b4a9262..ee7a3dc 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -97,6 +97,15 @@ config SATA_AHCI_PLATFORM
 
 	  If unsure, say N.
 
+config SATA_AHCI_ST
+	tristate "ST SATA support"
+	depends on SATA_AHCI_PLATFORM
+	select GENERIC_PHY
+	help
+	  This option enables support for ST SATA controller.
+
+	  If unsure, say N.
+
 config AHCI_IMX
 	tristate "Freescale i.MX AHCI SATA support"
 	depends on SATA_AHCI_PLATFORM && MFD_SYSCON
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 246050b..6bbd6da 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_ATA)		+= libata.o
 obj-$(CONFIG_SATA_AHCI)		+= ahci.o libahci.o
 obj-$(CONFIG_SATA_ACARD_AHCI)	+= acard-ahci.o libahci.o
 obj-$(CONFIG_SATA_AHCI_PLATFORM) += ahci_platform.o libahci.o
+obj-$(CONFIG_SATA_AHCI_ST)      += ahci_st.o
 obj-$(CONFIG_SATA_FSL)		+= sata_fsl.o
 obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
new file mode 100644
index 0000000..2f95133
--- /dev/null
+++ b/drivers/ata/ahci_st.c
@@ -0,0 +1,239 @@
+/*
+ * Copyright (C) 2012 STMicroelectronics Limited
+ *
+ * Authors: Francesco Virlinzi <francesco.virlinzi@st.com>
+ *	    Alexandre Torgue <alexandre.torgue@st.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/export.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/ahci_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/libata.h>
+#include <linux/reset.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+
+#include "ahci.h"
+
+#define ST_AHCI_OOBR			0xbc
+#define ST_AHCI_OOBR_WE			BIT(31)
+#define ST_AHCI_OOBR_CWMIN_SHIFT	24
+#define ST_AHCI_OOBR_CWMAX_SHIFT	16
+#define ST_AHCI_OOBR_CIMIN_SHIFT	8
+#define ST_AHCI_OOBR_CIMAX_SHIFT	0
+
+struct st_ahci_drv_data {
+	struct platform_device *ahci;
+	struct phy *phy;
+	struct reset_control *pwr;
+	struct reset_control *sw_rst;
+	struct reset_control *pwr_rst;
+	struct ahci_host_priv *hpriv;
+};
+
+static void st_ahci_configure_oob(void __iomem *mmio)
+{
+	unsigned long old_val, new_val;
+
+	new_val = (0x02 << ST_AHCI_OOBR_CWMIN_SHIFT) |
+		  (0x04 << ST_AHCI_OOBR_CWMAX_SHIFT) |
+		  (0x08 << ST_AHCI_OOBR_CIMIN_SHIFT) |
+		  (0x0C << ST_AHCI_OOBR_CIMAX_SHIFT);
+
+	old_val = readl(mmio + ST_AHCI_OOBR);
+	writel(old_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
+	writel(new_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
+	writel(new_val, mmio + ST_AHCI_OOBR);
+}
+
+static int st_ahci_deassert_resets(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_deassert(drv_data->pwr);
+		if (err) {
+			dev_err(dev, "unable to bring out of pwrdwn\n");
+			return err;
+		}
+	}
+
+	st_ahci_configure_oob(drv_data->hpriv->mmio);
+
+	if (drv_data->sw_rst) {
+		err = reset_control_deassert(drv_data->sw_rst);
+		if (err) {
+			dev_err(dev, "unable to bring out of sw-rst\n");
+			return err;
+		}
+	}
+
+	if (drv_data->pwr_rst) {
+		err = reset_control_deassert(drv_data->pwr_rst);
+		if (err) {
+			dev_err(dev, "unable to bring out of pwr-rst\n");
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int st_ahci_probe_resets(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+
+	drv_data->pwr = devm_reset_control_get(&pdev->dev, "pwr-dwn");
+	if (IS_ERR(drv_data->pwr)) {
+		dev_info(&pdev->dev, "power reset control not defined\n");
+		drv_data->pwr = NULL;
+	}
+
+	drv_data->sw_rst = devm_reset_control_get(&pdev->dev, "sw-rst");
+	if (IS_ERR(drv_data->sw_rst)) {
+		dev_info(&pdev->dev, "soft reset control not defined\n");
+		drv_data->sw_rst = NULL;
+	}
+
+	drv_data->pwr_rst = devm_reset_control_get(&pdev->dev, "pwr-rst");
+	if (IS_ERR(drv_data->pwr_rst)) {
+		dev_dbg(&pdev->dev, "power soft reset control not defined\n");
+		drv_data->pwr_rst = NULL;
+	}
+
+	return st_ahci_deassert_resets(&pdev->dev);
+}
+
+static const struct ata_port_info st_ahci_port_info = {
+	.flags          = AHCI_FLAG_COMMON,
+	.pio_mask       = ATA_PIO4,
+	.udma_mask      = ATA_UDMA6,
+	.port_ops       = &ahci_platform_ops,
+};
+
+static int st_ahci_probe(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data;
+	struct ahci_host_priv *hpriv;
+	int err;
+
+	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
+	if (!drv_data)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, drv_data);
+
+	hpriv = ahci_platform_get_resources(pdev);
+	if (IS_ERR(hpriv))
+		return PTR_ERR(hpriv);
+
+	drv_data->hpriv = hpriv;
+
+	err = st_ahci_probe_resets(pdev);
+	if (err)
+		return err;
+
+	err = ahci_platform_enable_resources(hpriv);
+	if (err)
+		return err;
+
+	err = ahci_platform_init_host(pdev, hpriv, &st_ahci_port_info, 0, 0);
+	if (err) {
+		ahci_platform_disable_resources(hpriv);
+		return err;
+	}
+
+	return 0;
+}
+
+static int st_ahci_remove(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_assert(drv_data->pwr);
+		if (err)
+			dev_err(&pdev->dev, "unable to pwrdwn\n");
+	}
+
+	ahci_platform_disable_resources(hpriv);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int st_ahci_suspend(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_assert(drv_data->pwr);
+		if (err) {
+			dev_err(dev, "unable to pwrdwn");
+			return err;
+		}
+	}
+
+	ahci_platform_disable_resources(hpriv);
+
+	return 0;
+}
+
+static int st_ahci_resume(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	err = ahci_platform_enable_resources(hpriv);
+	if (err)
+		return err;
+
+	err = st_ahci_deassert_resets(dev);
+	if (err) {
+		ahci_platform_disable_resources(hpriv);
+		return err;
+	}
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(st_ahci_pm_ops, st_ahci_suspend, st_ahci_resume);
+
+static struct of_device_id st_ahci_match[] = {
+	{ .compatible = "st,ahci", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, st_ahci_match);
+
+static struct platform_driver st_ahci_driver = {
+	.driver = {
+		.name = "st_ahci",
+		.owner = THIS_MODULE,
+		.pm = &st_ahci_pm_ops,
+		.of_match_table = of_match_ptr(st_ahci_match),
+	},
+	.probe = st_ahci_probe,
+	.remove = st_ahci_remove,
+};
+module_platform_driver(st_ahci_driver);
+
+MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@st.com>");
+MODULE_AUTHOR("Francesco Virlinzi <francesco.virlinzi@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics Sata Ahci driver");
+MODULE_LICENSE("GPL v2");

[PATCH v3 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

ahci: st: Add support for ST's SATA IP

Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index b4a9262..ee7a3dc 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -97,6 +97,15 @@ config SATA_AHCI_PLATFORM
 
 	  If unsure, say N.
 
+config SATA_AHCI_ST
+	tristate "ST SATA support"
+	depends on SATA_AHCI_PLATFORM
+	select GENERIC_PHY
+	help
+	  This option enables support for ST SATA controller.
+
+	  If unsure, say N.
+
 config AHCI_IMX
 	tristate "Freescale i.MX AHCI SATA support"
 	depends on SATA_AHCI_PLATFORM && MFD_SYSCON
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 246050b..6bbd6da 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_ATA)		+= libata.o
 obj-$(CONFIG_SATA_AHCI)		+= ahci.o libahci.o
 obj-$(CONFIG_SATA_ACARD_AHCI)	+= acard-ahci.o libahci.o
 obj-$(CONFIG_SATA_AHCI_PLATFORM) += ahci_platform.o libahci.o
+obj-$(CONFIG_SATA_AHCI_ST)      += ahci_st.o
 obj-$(CONFIG_SATA_FSL)		+= sata_fsl.o
 obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
new file mode 100644
index 0000000..2f95133
--- /dev/null
+++ b/drivers/ata/ahci_st.c
@@ -0,0 +1,239 @@
+/*
+ * Copyright (C) 2012 STMicroelectronics Limited
+ *
+ * Authors: Francesco Virlinzi <francesco.virlinzi@st.com>
+ *	    Alexandre Torgue <alexandre.torgue@st.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/export.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/ahci_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/libata.h>
+#include <linux/reset.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+
+#include "ahci.h"
+
+#define ST_AHCI_OOBR			0xbc
+#define ST_AHCI_OOBR_WE			BIT(31)
+#define ST_AHCI_OOBR_CWMIN_SHIFT	24
+#define ST_AHCI_OOBR_CWMAX_SHIFT	16
+#define ST_AHCI_OOBR_CIMIN_SHIFT	8
+#define ST_AHCI_OOBR_CIMAX_SHIFT	0
+
+struct st_ahci_drv_data {
+	struct platform_device *ahci;
+	struct phy *phy;
+	struct reset_control *pwr;
+	struct reset_control *sw_rst;
+	struct reset_control *pwr_rst;
+	struct ahci_host_priv *hpriv;
+};
+
+static void st_ahci_configure_oob(void __iomem *mmio)
+{
+	unsigned long old_val, new_val;
+
+	new_val = (0x02 << ST_AHCI_OOBR_CWMIN_SHIFT) |
+		  (0x04 << ST_AHCI_OOBR_CWMAX_SHIFT) |
+		  (0x08 << ST_AHCI_OOBR_CIMIN_SHIFT) |
+		  (0x0C << ST_AHCI_OOBR_CIMAX_SHIFT);
+
+	old_val = readl(mmio + ST_AHCI_OOBR);
+	writel(old_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
+	writel(new_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
+	writel(new_val, mmio + ST_AHCI_OOBR);
+}
+
+static int st_ahci_deassert_resets(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_deassert(drv_data->pwr);
+		if (err) {
+			dev_err(dev, "unable to bring out of pwrdwn\n");
+			return err;
+		}
+	}
+
+	st_ahci_configure_oob(drv_data->hpriv->mmio);
+
+	if (drv_data->sw_rst) {
+		err = reset_control_deassert(drv_data->sw_rst);
+		if (err) {
+			dev_err(dev, "unable to bring out of sw-rst\n");
+			return err;
+		}
+	}
+
+	if (drv_data->pwr_rst) {
+		err = reset_control_deassert(drv_data->pwr_rst);
+		if (err) {
+			dev_err(dev, "unable to bring out of pwr-rst\n");
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int st_ahci_probe_resets(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+
+	drv_data->pwr = devm_reset_control_get(&pdev->dev, "pwr-dwn");
+	if (IS_ERR(drv_data->pwr)) {
+		dev_info(&pdev->dev, "power reset control not defined\n");
+		drv_data->pwr = NULL;
+	}
+
+	drv_data->sw_rst = devm_reset_control_get(&pdev->dev, "sw-rst");
+	if (IS_ERR(drv_data->sw_rst)) {
+		dev_info(&pdev->dev, "soft reset control not defined\n");
+		drv_data->sw_rst = NULL;
+	}
+
+	drv_data->pwr_rst = devm_reset_control_get(&pdev->dev, "pwr-rst");
+	if (IS_ERR(drv_data->pwr_rst)) {
+		dev_dbg(&pdev->dev, "power soft reset control not defined\n");
+		drv_data->pwr_rst = NULL;
+	}
+
+	return st_ahci_deassert_resets(&pdev->dev);
+}
+
+static const struct ata_port_info st_ahci_port_info = {
+	.flags          = AHCI_FLAG_COMMON,
+	.pio_mask       = ATA_PIO4,
+	.udma_mask      = ATA_UDMA6,
+	.port_ops       = &ahci_platform_ops,
+};
+
+static int st_ahci_probe(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data;
+	struct ahci_host_priv *hpriv;
+	int err;
+
+	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
+	if (!drv_data)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, drv_data);
+
+	hpriv = ahci_platform_get_resources(pdev);
+	if (IS_ERR(hpriv))
+		return PTR_ERR(hpriv);
+
+	drv_data->hpriv = hpriv;
+
+	err = st_ahci_probe_resets(pdev);
+	if (err)
+		return err;
+
+	err = ahci_platform_enable_resources(hpriv);
+	if (err)
+		return err;
+
+	err = ahci_platform_init_host(pdev, hpriv, &st_ahci_port_info, 0, 0);
+	if (err) {
+		ahci_platform_disable_resources(hpriv);
+		return err;
+	}
+
+	return 0;
+}
+
+static int st_ahci_remove(struct platform_device *pdev)
+{
+	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_assert(drv_data->pwr);
+		if (err)
+			dev_err(&pdev->dev, "unable to pwrdwn\n");
+	}
+
+	ahci_platform_disable_resources(hpriv);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int st_ahci_suspend(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	if (drv_data->pwr) {
+		err = reset_control_assert(drv_data->pwr);
+		if (err) {
+			dev_err(dev, "unable to pwrdwn");
+			return err;
+		}
+	}
+
+	ahci_platform_disable_resources(hpriv);
+
+	return 0;
+}
+
+static int st_ahci_resume(struct device *dev)
+{
+	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = drv_data->hpriv;
+	int err;
+
+	err = ahci_platform_enable_resources(hpriv);
+	if (err)
+		return err;
+
+	err = st_ahci_deassert_resets(dev);
+	if (err) {
+		ahci_platform_disable_resources(hpriv);
+		return err;
+	}
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(st_ahci_pm_ops, st_ahci_suspend, st_ahci_resume);
+
+static struct of_device_id st_ahci_match[] = {
+	{ .compatible = "st,ahci", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, st_ahci_match);
+
+static struct platform_driver st_ahci_driver = {
+	.driver = {
+		.name = "st_ahci",
+		.owner = THIS_MODULE,
+		.pm = &st_ahci_pm_ops,
+		.of_match_table = of_match_ptr(st_ahci_match),
+	},
+	.probe = st_ahci_probe,
+	.remove = st_ahci_remove,
+};
+module_platform_driver(st_ahci_driver);
+
+MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@st.com>");
+MODULE_AUTHOR("Francesco Virlinzi <francesco.virlinzi@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics Sata Ahci driver");
+MODULE_LICENSE("GPL v2");

Re: [PATCH v3 3/3] ahci: st: Add support for ST's SATA IP

[Bartlomiej Zolnierkiewicz]

Hi,

On Wednesday, February 26, 2014 02:47:21 PM Lee Jones wrote:
> ahci: st: Add support for ST's SATA IP
> 
> Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index b4a9262..ee7a3dc 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -97,6 +97,15 @@ config SATA_AHCI_PLATFORM
>  
>  	  If unsure, say N.
>  
> +config SATA_AHCI_ST

It would be better to name it just ACHI_ST for consistency with
existing config options (AHCI_IMX and AHCI_SUNXI).

> +	tristate "ST SATA support"

"ST AHCI SATA support"

> +	depends on SATA_AHCI_PLATFORM

There should be also dependency on ARCH_STI here.  We don't want this
driver to be available on other ARM architectures.

> +	select GENERIC_PHY

This doesn't belong here anylonger and should be removed.

> +	help
> +	  This option enables support for ST SATA controller.

ST AHCI SATA

> +	  If unsure, say N.
> +
>  config AHCI_IMX
>  	tristate "Freescale i.MX AHCI SATA support"
>  	depends on SATA_AHCI_PLATFORM && MFD_SYSCON
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 246050b..6bbd6da 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_ATA)		+= libata.o
>  obj-$(CONFIG_SATA_AHCI)		+= ahci.o libahci.o
>  obj-$(CONFIG_SATA_ACARD_AHCI)	+= acard-ahci.o libahci.o
>  obj-$(CONFIG_SATA_AHCI_PLATFORM) += ahci_platform.o libahci.o
> +obj-$(CONFIG_SATA_AHCI_ST)      += ahci_st.o
>  obj-$(CONFIG_SATA_FSL)		+= sata_fsl.o
>  obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
>  obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
> diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
> new file mode 100644
> index 0000000..2f95133
> --- /dev/null
> +++ b/drivers/ata/ahci_st.c
> @@ -0,0 +1,239 @@
> +/*
> + * Copyright (C) 2012 STMicroelectronics Limited
> + *
> + * Authors: Francesco Virlinzi <francesco.virlinzi@st.com>
> + *	    Alexandre Torgue <alexandre.torgue@st.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/export.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/ahci_platform.h>
> +#include <linux/phy/phy.h>

Not needed anylonger.

> +#include <linux/libata.h>
> +#include <linux/reset.h>
> +#include <linux/io.h>
> +#include <linux/dma-mapping.h>
> +
> +#include "ahci.h"
> +
> +#define ST_AHCI_OOBR			0xbc
> +#define ST_AHCI_OOBR_WE			BIT(31)
> +#define ST_AHCI_OOBR_CWMIN_SHIFT	24
> +#define ST_AHCI_OOBR_CWMAX_SHIFT	16
> +#define ST_AHCI_OOBR_CIMIN_SHIFT	8
> +#define ST_AHCI_OOBR_CIMAX_SHIFT	0
> +
> +struct st_ahci_drv_data {
> +	struct platform_device *ahci;
> +	struct phy *phy;

ditto

> +	struct reset_control *pwr;
> +	struct reset_control *sw_rst;
> +	struct reset_control *pwr_rst;
> +	struct ahci_host_priv *hpriv;
> +};
> +
> +static void st_ahci_configure_oob(void __iomem *mmio)
> +{
> +	unsigned long old_val, new_val;
> +
> +	new_val = (0x02 << ST_AHCI_OOBR_CWMIN_SHIFT) |
> +		  (0x04 << ST_AHCI_OOBR_CWMAX_SHIFT) |
> +		  (0x08 << ST_AHCI_OOBR_CIMIN_SHIFT) |
> +		  (0x0C << ST_AHCI_OOBR_CIMAX_SHIFT);
> +
> +	old_val = readl(mmio + ST_AHCI_OOBR);
> +	writel(old_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
> +	writel(new_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
> +	writel(new_val, mmio + ST_AHCI_OOBR);
> +}
> +
> +static int st_ahci_deassert_resets(struct device *dev)
> +{
> +	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
> +	int err;
> +
> +	if (drv_data->pwr) {
> +		err = reset_control_deassert(drv_data->pwr);
> +		if (err) {
> +			dev_err(dev, "unable to bring out of pwrdwn\n");
> +			return err;
> +		}
> +	}
> +
> +	st_ahci_configure_oob(drv_data->hpriv->mmio);
> +
> +	if (drv_data->sw_rst) {
> +		err = reset_control_deassert(drv_data->sw_rst);
> +		if (err) {
> +			dev_err(dev, "unable to bring out of sw-rst\n");
> +			return err;
> +		}
> +	}
> +
> +	if (drv_data->pwr_rst) {
> +		err = reset_control_deassert(drv_data->pwr_rst);
> +		if (err) {
> +			dev_err(dev, "unable to bring out of pwr-rst\n");
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int st_ahci_probe_resets(struct platform_device *pdev)
> +{
> +	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
> +
> +	drv_data->pwr = devm_reset_control_get(&pdev->dev, "pwr-dwn");
> +	if (IS_ERR(drv_data->pwr)) {
> +		dev_info(&pdev->dev, "power reset control not defined\n");
> +		drv_data->pwr = NULL;
> +	}
> +
> +	drv_data->sw_rst = devm_reset_control_get(&pdev->dev, "sw-rst");
> +	if (IS_ERR(drv_data->sw_rst)) {
> +		dev_info(&pdev->dev, "soft reset control not defined\n");
> +		drv_data->sw_rst = NULL;
> +	}
> +
> +	drv_data->pwr_rst = devm_reset_control_get(&pdev->dev, "pwr-rst");
> +	if (IS_ERR(drv_data->pwr_rst)) {
> +		dev_dbg(&pdev->dev, "power soft reset control not defined\n");
> +		drv_data->pwr_rst = NULL;
> +	}
> +
> +	return st_ahci_deassert_resets(&pdev->dev);
> +}
> +
> +static const struct ata_port_info st_ahci_port_info = {
> +	.flags          = AHCI_FLAG_COMMON,
> +	.pio_mask       = ATA_PIO4,
> +	.udma_mask      = ATA_UDMA6,
> +	.port_ops       = &ahci_platform_ops,
> +};
> +
> +static int st_ahci_probe(struct platform_device *pdev)
> +{
> +	struct st_ahci_drv_data *drv_data;
> +	struct ahci_host_priv *hpriv;
> +	int err;
> +
> +	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
> +	if (!drv_data)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, drv_data);
> +
> +	hpriv = ahci_platform_get_resources(pdev);
> +	if (IS_ERR(hpriv))
> +		return PTR_ERR(hpriv);
> +
> +	drv_data->hpriv = hpriv;
> +
> +	err = st_ahci_probe_resets(pdev);
> +	if (err)
> +		return err;
> +
> +	err = ahci_platform_enable_resources(hpriv);
> +	if (err)
> +		return err;
> +
> +	err = ahci_platform_init_host(pdev, hpriv, &st_ahci_port_info, 0, 0);
> +	if (err) {
> +		ahci_platform_disable_resources(hpriv);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int st_ahci_remove(struct platform_device *pdev)
> +{
> +	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
> +	struct ahci_host_priv *hpriv = drv_data->hpriv;
> +	int err;
> +
> +	if (drv_data->pwr) {
> +		err = reset_control_assert(drv_data->pwr);
> +		if (err)
> +			dev_err(&pdev->dev, "unable to pwrdwn\n");
> +	}
> +
> +	ahci_platform_disable_resources(hpriv);
> +
> +	return 0;
> +}

This driver should define its own ->host_stop method which will
do reset_control_assert() and ahci_platform_disable_resources().

Then instead of st_ahci_remove() the ata_platform_remove_one()
one should be used as ->remove method.

[ Please see ahci_imx.c for details/example. ]

> +#ifdef CONFIG_PM_SLEEP
> +static int st_ahci_suspend(struct device *dev)
> +{
> +	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
> +	struct ahci_host_priv *hpriv = drv_data->hpriv;
> +	int err;
> +
> +	if (drv_data->pwr) {
> +		err = reset_control_assert(drv_data->pwr);
> +		if (err) {
> +			dev_err(dev, "unable to pwrdwn");
> +			return err;
> +		}
> +	}
> +
> +	ahci_platform_disable_resources(hpriv);

I think that ata_platform_suspend_host() should be used in
st_ahci_suspend() (please refer to ahci_imx.c).

> +	return 0;
> +}
> +
> +static int st_ahci_resume(struct device *dev)
> +{
> +	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
> +	struct ahci_host_priv *hpriv = drv_data->hpriv;
> +	int err;
> +
> +	err = ahci_platform_enable_resources(hpriv);

Similarly, ata_platform_resume_host() should be used in
st_ahci_resume() (please refer to ahci_imx.c).

> +	if (err)
> +		return err;
> +
> +	err = st_ahci_deassert_resets(dev);
> +	if (err) {
> +		ahci_platform_disable_resources(hpriv);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(st_ahci_pm_ops, st_ahci_suspend, st_ahci_resume);
> +
> +static struct of_device_id st_ahci_match[] = {
> +	{ .compatible = "st,ahci", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, st_ahci_match);
> +
> +static struct platform_driver st_ahci_driver = {
> +	.driver = {
> +		.name = "st_ahci",
> +		.owner = THIS_MODULE,
> +		.pm = &st_ahci_pm_ops,
> +		.of_match_table = of_match_ptr(st_ahci_match),
> +	},
> +	.probe = st_ahci_probe,
> +	.remove = st_ahci_remove,
> +};
> +module_platform_driver(st_ahci_driver);
> +
> +MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@st.com>");
> +MODULE_AUTHOR("Francesco Virlinzi <francesco.virlinzi@st.com>");
> +MODULE_DESCRIPTION("STMicroelectronics Sata Ahci driver");

"Sata Ahci" -> "SATA AHCI"

> +MODULE_LICENSE("GPL v2");

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH v3 3/3] ahci: st: Add support for ST's SATA IP

[Bartlomiej Zolnierkiewicz]

Hi,

On Wednesday, February 26, 2014 02:47:21 PM Lee Jones wrote:
> ahci: st: Add support for ST's SATA IP
> 
> Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index b4a9262..ee7a3dc 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -97,6 +97,15 @@ config SATA_AHCI_PLATFORM
>  
>  	  If unsure, say N.
>  
> +config SATA_AHCI_ST

It would be better to name it just ACHI_ST for consistency with
existing config options (AHCI_IMX and AHCI_SUNXI).

> +	tristate "ST SATA support"

"ST AHCI SATA support"

> +	depends on SATA_AHCI_PLATFORM

There should be also dependency on ARCH_STI here.  We don't want this
driver to be available on other ARM architectures.

> +	select GENERIC_PHY

This doesn't belong here anylonger and should be removed.

> +	help
> +	  This option enables support for ST SATA controller.

ST AHCI SATA

> +	  If unsure, say N.
> +
>  config AHCI_IMX
>  	tristate "Freescale i.MX AHCI SATA support"
>  	depends on SATA_AHCI_PLATFORM && MFD_SYSCON
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 246050b..6bbd6da 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_ATA)		+= libata.o
>  obj-$(CONFIG_SATA_AHCI)		+= ahci.o libahci.o
>  obj-$(CONFIG_SATA_ACARD_AHCI)	+= acard-ahci.o libahci.o
>  obj-$(CONFIG_SATA_AHCI_PLATFORM) += ahci_platform.o libahci.o
> +obj-$(CONFIG_SATA_AHCI_ST)      += ahci_st.o
>  obj-$(CONFIG_SATA_FSL)		+= sata_fsl.o
>  obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
>  obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
> diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
> new file mode 100644
> index 0000000..2f95133
> --- /dev/null
> +++ b/drivers/ata/ahci_st.c
> @@ -0,0 +1,239 @@
> +/*
> + * Copyright (C) 2012 STMicroelectronics Limited
> + *
> + * Authors: Francesco Virlinzi <francesco.virlinzi@st.com>
> + *	    Alexandre Torgue <alexandre.torgue@st.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/export.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/ahci_platform.h>
> +#include <linux/phy/phy.h>

Not needed anylonger.

> +#include <linux/libata.h>
> +#include <linux/reset.h>
> +#include <linux/io.h>
> +#include <linux/dma-mapping.h>
> +
> +#include "ahci.h"
> +
> +#define ST_AHCI_OOBR			0xbc
> +#define ST_AHCI_OOBR_WE			BIT(31)
> +#define ST_AHCI_OOBR_CWMIN_SHIFT	24
> +#define ST_AHCI_OOBR_CWMAX_SHIFT	16
> +#define ST_AHCI_OOBR_CIMIN_SHIFT	8
> +#define ST_AHCI_OOBR_CIMAX_SHIFT	0
> +
> +struct st_ahci_drv_data {
> +	struct platform_device *ahci;
> +	struct phy *phy;

ditto

> +	struct reset_control *pwr;
> +	struct reset_control *sw_rst;
> +	struct reset_control *pwr_rst;
> +	struct ahci_host_priv *hpriv;
> +};
> +
> +static void st_ahci_configure_oob(void __iomem *mmio)
> +{
> +	unsigned long old_val, new_val;
> +
> +	new_val = (0x02 << ST_AHCI_OOBR_CWMIN_SHIFT) |
> +		  (0x04 << ST_AHCI_OOBR_CWMAX_SHIFT) |
> +		  (0x08 << ST_AHCI_OOBR_CIMIN_SHIFT) |
> +		  (0x0C << ST_AHCI_OOBR_CIMAX_SHIFT);
> +
> +	old_val = readl(mmio + ST_AHCI_OOBR);
> +	writel(old_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
> +	writel(new_val | ST_AHCI_OOBR_WE, mmio + ST_AHCI_OOBR);
> +	writel(new_val, mmio + ST_AHCI_OOBR);
> +}
> +
> +static int st_ahci_deassert_resets(struct device *dev)
> +{
> +	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
> +	int err;
> +
> +	if (drv_data->pwr) {
> +		err = reset_control_deassert(drv_data->pwr);
> +		if (err) {
> +			dev_err(dev, "unable to bring out of pwrdwn\n");
> +			return err;
> +		}
> +	}
> +
> +	st_ahci_configure_oob(drv_data->hpriv->mmio);
> +
> +	if (drv_data->sw_rst) {
> +		err = reset_control_deassert(drv_data->sw_rst);
> +		if (err) {
> +			dev_err(dev, "unable to bring out of sw-rst\n");
> +			return err;
> +		}
> +	}
> +
> +	if (drv_data->pwr_rst) {
> +		err = reset_control_deassert(drv_data->pwr_rst);
> +		if (err) {
> +			dev_err(dev, "unable to bring out of pwr-rst\n");
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int st_ahci_probe_resets(struct platform_device *pdev)
> +{
> +	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
> +
> +	drv_data->pwr = devm_reset_control_get(&pdev->dev, "pwr-dwn");
> +	if (IS_ERR(drv_data->pwr)) {
> +		dev_info(&pdev->dev, "power reset control not defined\n");
> +		drv_data->pwr = NULL;
> +	}
> +
> +	drv_data->sw_rst = devm_reset_control_get(&pdev->dev, "sw-rst");
> +	if (IS_ERR(drv_data->sw_rst)) {
> +		dev_info(&pdev->dev, "soft reset control not defined\n");
> +		drv_data->sw_rst = NULL;
> +	}
> +
> +	drv_data->pwr_rst = devm_reset_control_get(&pdev->dev, "pwr-rst");
> +	if (IS_ERR(drv_data->pwr_rst)) {
> +		dev_dbg(&pdev->dev, "power soft reset control not defined\n");
> +		drv_data->pwr_rst = NULL;
> +	}
> +
> +	return st_ahci_deassert_resets(&pdev->dev);
> +}
> +
> +static const struct ata_port_info st_ahci_port_info = {
> +	.flags          = AHCI_FLAG_COMMON,
> +	.pio_mask       = ATA_PIO4,
> +	.udma_mask      = ATA_UDMA6,
> +	.port_ops       = &ahci_platform_ops,
> +};
> +
> +static int st_ahci_probe(struct platform_device *pdev)
> +{
> +	struct st_ahci_drv_data *drv_data;
> +	struct ahci_host_priv *hpriv;
> +	int err;
> +
> +	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
> +	if (!drv_data)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, drv_data);
> +
> +	hpriv = ahci_platform_get_resources(pdev);
> +	if (IS_ERR(hpriv))
> +		return PTR_ERR(hpriv);
> +
> +	drv_data->hpriv = hpriv;
> +
> +	err = st_ahci_probe_resets(pdev);
> +	if (err)
> +		return err;
> +
> +	err = ahci_platform_enable_resources(hpriv);
> +	if (err)
> +		return err;
> +
> +	err = ahci_platform_init_host(pdev, hpriv, &st_ahci_port_info, 0, 0);
> +	if (err) {
> +		ahci_platform_disable_resources(hpriv);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int st_ahci_remove(struct platform_device *pdev)
> +{
> +	struct st_ahci_drv_data *drv_data = platform_get_drvdata(pdev);
> +	struct ahci_host_priv *hpriv = drv_data->hpriv;
> +	int err;
> +
> +	if (drv_data->pwr) {
> +		err = reset_control_assert(drv_data->pwr);
> +		if (err)
> +			dev_err(&pdev->dev, "unable to pwrdwn\n");
> +	}
> +
> +	ahci_platform_disable_resources(hpriv);
> +
> +	return 0;
> +}

This driver should define its own ->host_stop method which will
do reset_control_assert() and ahci_platform_disable_resources().

Then instead of st_ahci_remove() the ata_platform_remove_one()
one should be used as ->remove method.

[ Please see ahci_imx.c for details/example. ]

> +#ifdef CONFIG_PM_SLEEP
> +static int st_ahci_suspend(struct device *dev)
> +{
> +	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
> +	struct ahci_host_priv *hpriv = drv_data->hpriv;
> +	int err;
> +
> +	if (drv_data->pwr) {
> +		err = reset_control_assert(drv_data->pwr);
> +		if (err) {
> +			dev_err(dev, "unable to pwrdwn");
> +			return err;
> +		}
> +	}
> +
> +	ahci_platform_disable_resources(hpriv);

I think that ata_platform_suspend_host() should be used in
st_ahci_suspend() (please refer to ahci_imx.c).

> +	return 0;
> +}
> +
> +static int st_ahci_resume(struct device *dev)
> +{
> +	struct st_ahci_drv_data *drv_data = dev_get_drvdata(dev);
> +	struct ahci_host_priv *hpriv = drv_data->hpriv;
> +	int err;
> +
> +	err = ahci_platform_enable_resources(hpriv);

Similarly, ata_platform_resume_host() should be used in
st_ahci_resume() (please refer to ahci_imx.c).

> +	if (err)
> +		return err;
> +
> +	err = st_ahci_deassert_resets(dev);
> +	if (err) {
> +		ahci_platform_disable_resources(hpriv);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(st_ahci_pm_ops, st_ahci_suspend, st_ahci_resume);
> +
> +static struct of_device_id st_ahci_match[] = {
> +	{ .compatible = "st,ahci", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, st_ahci_match);
> +
> +static struct platform_driver st_ahci_driver = {
> +	.driver = {
> +		.name = "st_ahci",
> +		.owner = THIS_MODULE,
> +		.pm = &st_ahci_pm_ops,
> +		.of_match_table = of_match_ptr(st_ahci_match),
> +	},
> +	.probe = st_ahci_probe,
> +	.remove = st_ahci_remove,
> +};
> +module_platform_driver(st_ahci_driver);
> +
> +MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@st.com>");
> +MODULE_AUTHOR("Francesco Virlinzi <francesco.virlinzi@st.com>");
> +MODULE_DESCRIPTION("STMicroelectronics Sata Ahci driver");

"Sata Ahci" -> "SATA AHCI"

> +MODULE_LICENSE("GPL v2");

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH v3 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

On Tue, Mar 04, 2014 at 02:48:54PM +0100, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Wednesday, February 26, 2014 02:47:21 PM Lee Jones wrote:
> > ahci: st: Add support for ST's SATA IP
> > 
> > Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> > index b4a9262..ee7a3dc 100644
> > --- a/drivers/ata/Kconfig
> > +++ b/drivers/ata/Kconfig
> > @@ -97,6 +97,15 @@ config SATA_AHCI_PLATFORM
> >  
> >  	  If unsure, say N.
> >  
> > +config SATA_AHCI_ST
> 
> It would be better to name it just ACHI_ST for consistency with
> existing config options (AHCI_IMX and AHCI_SUNXI).

The patch is already in for-3.15 branch.  Lee, can you please post an
incremental patch addressing Bart's review points?

Thanks.

-- 
tejun

[PATCH v3 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

On Tue, Mar 04, 2014 at 02:48:54PM +0100, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Wednesday, February 26, 2014 02:47:21 PM Lee Jones wrote:
> > ahci: st: Add support for ST's SATA IP
> > 
> > Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> > index b4a9262..ee7a3dc 100644
> > --- a/drivers/ata/Kconfig
> > +++ b/drivers/ata/Kconfig
> > @@ -97,6 +97,15 @@ config SATA_AHCI_PLATFORM
> >  
> >  	  If unsure, say N.
> >  
> > +config SATA_AHCI_ST
> 
> It would be better to name it just ACHI_ST for consistency with
> existing config options (AHCI_IMX and AHCI_SUNXI).

The patch is already in for-3.15 branch.  Lee, can you please post an
incremental patch addressing Bart's review points?

Thanks.

-- 
tejun

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Mark Rutland]

On Tue, Feb 25, 2014 at 11:41:19AM +0000, Lee Jones wrote:
> Cc: devicetree@vger.kernel.org
> Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  Documentation/devicetree/bindings/ata/ahci-st.txt | 31 +++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt
> new file mode 100644
> index 0000000..1b69fa9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-st.txt
> @@ -0,0 +1,31 @@
> +STMicroelectronics STi SATA controller

Is that a family name or a particular device?

> +
> +This binding describes a SATA device.
> +
> +Required properties:
> + - compatible	   : Must be "st,ahci"

Is there not a more specific name? This is awfully generic.

> + - reg		   : Physical base addresses and length of register sets
> + - interrupts	   : Interrupt associated with the SATA device
> + - interrupt-names :   Associated name must be; "hostc"
> + - resets	   : The power-down and soft-reset lines of SATA IP
> + - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"

Get rid of the semicolon, it makes this harder to read. Same for the
other -names proeprties.

> + - clocks	   : The phandle for the clock
> + - clock-names	   :   Associated name must be; "ahci_clk"
> + - phys		   : The phandle for the PHY device
> + - phy-names	   :   Associated name must be; "ahci_phy"

This looks sane, but I'd like to see a more specific description and
compatible string.

Cheers,
Mark.

> +
> +Example:
> +
> +	sata0: sata@fe380000 {
> +		compatible      = "st,ahci";
> +		reg             = <0xfe380000 0x1000>;
> +		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
> +		interrupt-names = "hostc";
> +		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
> +		phy-names       = "ahci_phy";
> +		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
> +				  <&softreset STIH416_SATA0_SOFTRESET>;
> +		reset-names     = "pwr-dwn", "sw-rst";
> +		clocks	        = <&CLK_S_ICN_REG_0>;
> +		clock-names     = "ahci_clk";
> +	};
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Mark Rutland]

On Tue, Feb 25, 2014 at 11:41:19AM +0000, Lee Jones wrote:
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
> Acked-by: Alexandre Torgue <alexandre.torgue-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/ata/ahci-st.txt | 31 +++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt
> new file mode 100644
> index 0000000..1b69fa9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-st.txt
> @@ -0,0 +1,31 @@
> +STMicroelectronics STi SATA controller

Is that a family name or a particular device?

> +
> +This binding describes a SATA device.
> +
> +Required properties:
> + - compatible	   : Must be "st,ahci"

Is there not a more specific name? This is awfully generic.

> + - reg		   : Physical base addresses and length of register sets
> + - interrupts	   : Interrupt associated with the SATA device
> + - interrupt-names :   Associated name must be; "hostc"
> + - resets	   : The power-down and soft-reset lines of SATA IP
> + - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"

Get rid of the semicolon, it makes this harder to read. Same for the
other -names proeprties.

> + - clocks	   : The phandle for the clock
> + - clock-names	   :   Associated name must be; "ahci_clk"
> + - phys		   : The phandle for the PHY device
> + - phy-names	   :   Associated name must be; "ahci_phy"

This looks sane, but I'd like to see a more specific description and
compatible string.

Cheers,
Mark.

> +
> +Example:
> +
> +	sata0: sata@fe380000 {
> +		compatible      = "st,ahci";
> +		reg             = <0xfe380000 0x1000>;
> +		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
> +		interrupt-names = "hostc";
> +		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
> +		phy-names       = "ahci_phy";
> +		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
> +				  <&softreset STIH416_SATA0_SOFTRESET>;
> +		reset-names     = "pwr-dwn", "sw-rst";
> +		clocks	        = <&CLK_S_ICN_REG_0>;
> +		clock-names     = "ahci_clk";
> +	};
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Mark Rutland]

On Tue, Feb 25, 2014 at 11:41:19AM +0000, Lee Jones wrote:
> Cc: devicetree at vger.kernel.org
> Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  Documentation/devicetree/bindings/ata/ahci-st.txt | 31 +++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt
> new file mode 100644
> index 0000000..1b69fa9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-st.txt
> @@ -0,0 +1,31 @@
> +STMicroelectronics STi SATA controller

Is that a family name or a particular device?

> +
> +This binding describes a SATA device.
> +
> +Required properties:
> + - compatible	   : Must be "st,ahci"

Is there not a more specific name? This is awfully generic.

> + - reg		   : Physical base addresses and length of register sets
> + - interrupts	   : Interrupt associated with the SATA device
> + - interrupt-names :   Associated name must be; "hostc"
> + - resets	   : The power-down and soft-reset lines of SATA IP
> + - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"

Get rid of the semicolon, it makes this harder to read. Same for the
other -names proeprties.

> + - clocks	   : The phandle for the clock
> + - clock-names	   :   Associated name must be; "ahci_clk"
> + - phys		   : The phandle for the PHY device
> + - phy-names	   :   Associated name must be; "ahci_phy"

This looks sane, but I'd like to see a more specific description and
compatible string.

Cheers,
Mark.

> +
> +Example:
> +
> +	sata0: sata at fe380000 {
> +		compatible      = "st,ahci";
> +		reg             = <0xfe380000 0x1000>;
> +		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
> +		interrupt-names = "hostc";
> +		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
> +		phy-names       = "ahci_phy";
> +		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
> +				  <&softreset STIH416_SATA0_SOFTRESET>;
> +		reset-names     = "pwr-dwn", "sw-rst";
> +		clocks	        = <&CLK_S_ICN_REG_0>;
> +		clock-names     = "ahci_clk";
> +	};
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

> > +This binding describes a SATA device.
> > +
> > +Required properties:
> > + - compatible	   : Must be "st,ahci"
> 
> Is there not a more specific name? This is awfully generic.

There isn't one. ST only have one AHCI driver. The driver is named as
generically.

> > + - reg		   : Physical base addresses and length of register sets
> > + - interrupts	   : Interrupt associated with the SATA device
> > + - interrupt-names :   Associated name must be; "hostc"
> > + - resets	   : The power-down and soft-reset lines of SATA IP
> > + - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"
> 
> Get rid of the semicolon, it makes this harder to read. Same for the
> other -names proeprties.

Sure.

> > + - clocks	   : The phandle for the clock
> > + - clock-names	   :   Associated name must be; "ahci_clk"
> > + - phys		   : The phandle for the PHY device
> > + - phy-names	   :   Associated name must be; "ahci_phy"
> 
> This looks sane, but I'd like to see a more specific description and
> compatible string.

Specific description in what sense?

"This binding describes a SATA device."

to

"This binding describes ST's SATA device." ?

> > +Example:
> > +
> > +	sata0: sata@fe380000 {
> > +		compatible      = "st,ahci";
> > +		reg             = <0xfe380000 0x1000>;
> > +		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
> > +		interrupt-names = "hostc";
> > +		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
> > +		phy-names       = "ahci_phy";
> > +		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
> > +				  <&softreset STIH416_SATA0_SOFTRESET>;
> > +		reset-names     = "pwr-dwn", "sw-rst";
> > +		clocks	        = <&CLK_S_ICN_REG_0>;
> > +		clock-names     = "ahci_clk";
> > +	};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

> > +This binding describes a SATA device.
> > +
> > +Required properties:
> > + - compatible	   : Must be "st,ahci"
> 
> Is there not a more specific name? This is awfully generic.

There isn't one. ST only have one AHCI driver. The driver is named as
generically.

> > + - reg		   : Physical base addresses and length of register sets
> > + - interrupts	   : Interrupt associated with the SATA device
> > + - interrupt-names :   Associated name must be; "hostc"
> > + - resets	   : The power-down and soft-reset lines of SATA IP
> > + - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"
> 
> Get rid of the semicolon, it makes this harder to read. Same for the
> other -names proeprties.

Sure.

> > + - clocks	   : The phandle for the clock
> > + - clock-names	   :   Associated name must be; "ahci_clk"
> > + - phys		   : The phandle for the PHY device
> > + - phy-names	   :   Associated name must be; "ahci_phy"
> 
> This looks sane, but I'd like to see a more specific description and
> compatible string.

Specific description in what sense?

"This binding describes a SATA device."

to

"This binding describes ST's SATA device." ?

> > +Example:
> > +
> > +	sata0: sata@fe380000 {
> > +		compatible      = "st,ahci";
> > +		reg             = <0xfe380000 0x1000>;
> > +		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
> > +		interrupt-names = "hostc";
> > +		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
> > +		phy-names       = "ahci_phy";
> > +		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
> > +				  <&softreset STIH416_SATA0_SOFTRESET>;
> > +		reset-names     = "pwr-dwn", "sw-rst";
> > +		clocks	        = <&CLK_S_ICN_REG_0>;
> > +		clock-names     = "ahci_clk";
> > +	};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

> > +This binding describes a SATA device.
> > +
> > +Required properties:
> > + - compatible	   : Must be "st,ahci"
> 
> Is there not a more specific name? This is awfully generic.

There isn't one. ST only have one AHCI driver. The driver is named as
generically.

> > + - reg		   : Physical base addresses and length of register sets
> > + - interrupts	   : Interrupt associated with the SATA device
> > + - interrupt-names :   Associated name must be; "hostc"
> > + - resets	   : The power-down and soft-reset lines of SATA IP
> > + - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"
> 
> Get rid of the semicolon, it makes this harder to read. Same for the
> other -names proeprties.

Sure.

> > + - clocks	   : The phandle for the clock
> > + - clock-names	   :   Associated name must be; "ahci_clk"
> > + - phys		   : The phandle for the PHY device
> > + - phy-names	   :   Associated name must be; "ahci_phy"
> 
> This looks sane, but I'd like to see a more specific description and
> compatible string.

Specific description in what sense?

"This binding describes a SATA device."

to

"This binding describes ST's SATA device." ?

> > +Example:
> > +
> > +	sata0: sata at fe380000 {
> > +		compatible      = "st,ahci";
> > +		reg             = <0xfe380000 0x1000>;
> > +		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
> > +		interrupt-names = "hostc";
> > +		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
> > +		phy-names       = "ahci_phy";
> > +		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
> > +				  <&softreset STIH416_SATA0_SOFTRESET>;
> > +		reset-names     = "pwr-dwn", "sw-rst";
> > +		clocks	        = <&CLK_S_ICN_REG_0>;
> > +		clock-names     = "ahci_clk";
> > +	};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Arnd Bergmann]

On Wednesday 12 March 2014 12:48:35 Lee Jones wrote:
> > > +This binding describes a SATA device.
> > > +
> > > +Required properties:
> > > + - compatible         : Must be "st,ahci"
> > 
> > Is there not a more specific name? This is awfully generic.
> 
> There isn't one. ST only have one AHCI driver. The driver is named as
> generically.

If it is this generic, can't you use the completely generic
ahci-platform driver and add a "generic-ahci" compatible string
to that?

	Arnd

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Arnd Bergmann]

On Wednesday 12 March 2014 12:48:35 Lee Jones wrote:
> > > +This binding describes a SATA device.
> > > +
> > > +Required properties:
> > > + - compatible         : Must be "st,ahci"
> > 
> > Is there not a more specific name? This is awfully generic.
> 
> There isn't one. ST only have one AHCI driver. The driver is named as
> generically.

If it is this generic, can't you use the completely generic
ahci-platform driver and add a "generic-ahci" compatible string
to that?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Arnd Bergmann]

On Wednesday 12 March 2014 12:48:35 Lee Jones wrote:
> > > +This binding describes a SATA device.
> > > +
> > > +Required properties:
> > > + - compatible         : Must be "st,ahci"
> > 
> > Is there not a more specific name? This is awfully generic.
> 
> There isn't one. ST only have one AHCI driver. The driver is named as
> generically.

If it is this generic, can't you use the completely generic
ahci-platform driver and add a "generic-ahci" compatible string
to that?

	Arnd

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

> > > > +This binding describes a SATA device.
> > > > +
> > > > +Required properties:
> > > > + - compatible         : Must be "st,ahci"
> > > 
> > > Is there not a more specific name? This is awfully generic.
> > 
> > There isn't one. ST only have one AHCI driver. The driver is named as
> > generically.
> 
> If it is this generic, can't you use the completely generic
> ahci-platform driver and add a "generic-ahci" compatible string
> to that?

It pretty much does. The only 'special' functionality we handle in
the driver are resets and configuring the OOB. However, both are
required.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

> > > > +This binding describes a SATA device.
> > > > +
> > > > +Required properties:
> > > > + - compatible         : Must be "st,ahci"
> > > 
> > > Is there not a more specific name? This is awfully generic.
> > 
> > There isn't one. ST only have one AHCI driver. The driver is named as
> > generically.
> 
> If it is this generic, can't you use the completely generic
> ahci-platform driver and add a "generic-ahci" compatible string
> to that?

It pretty much does. The only 'special' functionality we handle in
the driver are resets and configuring the OOB. However, both are
required.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

> > > > +This binding describes a SATA device.
> > > > +
> > > > +Required properties:
> > > > + - compatible         : Must be "st,ahci"
> > > 
> > > Is there not a more specific name? This is awfully generic.
> > 
> > There isn't one. ST only have one AHCI driver. The driver is named as
> > generically.
> 
> If it is this generic, can't you use the completely generic
> ahci-platform driver and add a "generic-ahci" compatible string
> to that?

It pretty much does. The only 'special' functionality we handle in
the driver are resets and configuring the OOB. However, both are
required.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Arnd Bergmann]

On Wednesday 12 March 2014 17:14:52 Lee Jones wrote:
> > > > > +This binding describes a SATA device.
> > > > > +
> > > > > +Required properties:
> > > > > + - compatible         : Must be "st,ahci"
> > > > 
> > > > Is there not a more specific name? This is awfully generic.
> > > 
> > > There isn't one. ST only have one AHCI driver. The driver is named as
> > > generically.
> > 
> > If it is this generic, can't you use the completely generic
> > ahci-platform driver and add a "generic-ahci" compatible string
> > to that?
> 
> It pretty much does. The only 'special' functionality we handle in
> the driver are resets and configuring the OOB. However, both are
> required.

The reset handling should be easy to add to the generic driver
usign as an optional property if I'm not mistaken.

What is OOB?

	Arnd

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Arnd Bergmann]

On Wednesday 12 March 2014 17:14:52 Lee Jones wrote:
> > > > > +This binding describes a SATA device.
> > > > > +
> > > > > +Required properties:
> > > > > + - compatible         : Must be "st,ahci"
> > > > 
> > > > Is there not a more specific name? This is awfully generic.
> > > 
> > > There isn't one. ST only have one AHCI driver. The driver is named as
> > > generically.
> > 
> > If it is this generic, can't you use the completely generic
> > ahci-platform driver and add a "generic-ahci" compatible string
> > to that?
> 
> It pretty much does. The only 'special' functionality we handle in
> the driver are resets and configuring the OOB. However, both are
> required.

The reset handling should be easy to add to the generic driver
usign as an optional property if I'm not mistaken.

What is OOB?

	Arnd

[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Arnd Bergmann]

On Wednesday 12 March 2014 17:14:52 Lee Jones wrote:
> > > > > +This binding describes a SATA device.
> > > > > +
> > > > > +Required properties:
> > > > > + - compatible         : Must be "st,ahci"
> > > > 
> > > > Is there not a more specific name? This is awfully generic.
> > > 
> > > There isn't one. ST only have one AHCI driver. The driver is named as
> > > generically.
> > 
> > If it is this generic, can't you use the completely generic
> > ahci-platform driver and add a "generic-ahci" compatible string
> > to that?
> 
> It pretty much does. The only 'special' functionality we handle in
> the driver are resets and configuring the OOB. However, both are
> required.

The reset handling should be easy to add to the generic driver
usign as an optional property if I'm not mistaken.

What is OOB?

	Arnd