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

[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

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

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

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

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