[PATCH v2 0/3] add CNS3xxx AHCI support

[PATCH v2 0/3] add CNS3xxx AHCI support

[mkl0301 at gmail.com]

From: Mac Lin <mkl0301@gmail.com>

v2:
 - Switch ahci_platform to module device table matching to add SoC specific support

v1: http://www.spinics.net/lists/arm-kernel/msg106236.html
 - Add CNS3xxx SoC specific AHCI support

This patchset is based on linux-2.6.37-rc2

Mac Lin (3):
      ahci_platform: rename to ahci_pltfm, but keep the original module name
      ahci_pltfm: switch to module device table matching
      ahci_platform: add support for CNS3xxx SoC devices

 arch/arm/mach-cns3xxx/devices.c |    2 +-
 drivers/ata/Kconfig             |   11 ++
 drivers/ata/Makefile            |    5 +-
 drivers/ata/ahci_cns3xxx.c      |   62 +++++++++++
 drivers/ata/ahci_platform.c     |  197 ------------------------------------
 drivers/ata/ahci_pltfm.c        |  212 +++++++++++++++++++++++++++++++++++++++
 drivers/ata/ahci_pltfm.h        |   19 ++++
 7 files changed, 309 insertions(+), 199 deletions(-)
 create mode 100644 drivers/ata/ahci_cns3xxx.c
 delete mode 100644 drivers/ata/ahci_platform.c
 create mode 100644 drivers/ata/ahci_pltfm.c
 create mode 100644 drivers/ata/ahci_pltfm.h

[PATCH v2 1/3] ahci_platform: rename to ahci_pltfm, but keep the original module name

[mkl0301 at gmail.com]

From: Mac Lin <mkl0301@gmail.com>

Due to build system limitations, intermediate and final objects can't have the
same names. And as we're going to start building SoC-specific objects, rename
ahci_platform to ahci_pltfm to be linked with SoC-specific objects into
ahci_platform.

Signed-off-by: Mac Lin <mkl0301@gmail.com>
---
 drivers/ata/Makefile        |    4 +-
 drivers/ata/ahci_platform.c |  197 -------------------------------------------
 drivers/ata/ahci_pltfm.c    |  197 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 200 insertions(+), 198 deletions(-)
 delete mode 100644 drivers/ata/ahci_platform.c
 create mode 100644 drivers/ata/ahci_pltfm.c

diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 2b67c90..5b62be8 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -3,7 +3,6 @@ obj-$(CONFIG_ATA)		+= libata.o
 
 # non-SFF interface
 obj-$(CONFIG_SATA_AHCI)		+= ahci.o libahci.o
-obj-$(CONFIG_SATA_AHCI_PLATFORM) += ahci_platform.o libahci.o
 obj-$(CONFIG_SATA_FSL)		+= sata_fsl.o
 obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
@@ -99,6 +98,9 @@ obj-$(CONFIG_ATA_GENERIC)	+= ata_generic.o
 # Should be last libata driver
 obj-$(CONFIG_PATA_LEGACY)	+= pata_legacy.o
 
+obj-$(CONFIG_SATA_AHCI_PLATFORM) 		+= ahci_platform.o libahci.o
+ahci_platform-y					:= ahci_pltfm.o
+
 libata-y	:= libata-core.o libata-scsi.o libata-eh.o libata-transport.o
 libata-$(CONFIG_ATA_SFF)	+= libata-sff.o
 libata-$(CONFIG_SATA_PMP)	+= libata-pmp.o
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
deleted file mode 100644
index 6fef1fa..0000000
--- a/drivers/ata/ahci_platform.c
+++ /dev/null
@@ -1,197 +0,0 @@
-/*
- * AHCI SATA platform driver
- *
- * Copyright 2004-2005  Red Hat, Inc.
- *   Jeff Garzik <jgarzik@pobox.com>
- * Copyright 2010  MontaVista Software, LLC.
- *   Anton Vorontsov <avorontsov@ru.mvista.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2, or (at your option)
- * any later version.
- */
-
-#include <linux/kernel.h>
-#include <linux/gfp.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/interrupt.h>
-#include <linux/device.h>
-#include <linux/platform_device.h>
-#include <linux/libata.h>
-#include <linux/ahci_platform.h>
-#include "ahci.h"
-
-static struct scsi_host_template ahci_platform_sht = {
-	AHCI_SHT("ahci_platform"),
-};
-
-static int __init ahci_probe(struct platform_device *pdev)
-{
-	struct device *dev = &pdev->dev;
-	struct ahci_platform_data *pdata = dev->platform_data;
-	struct ata_port_info pi = {
-		.flags		= AHCI_FLAG_COMMON,
-		.pio_mask	= ATA_PIO4,
-		.udma_mask	= ATA_UDMA6,
-		.port_ops	= &ahci_ops,
-	};
-	const struct ata_port_info *ppi[] = { &pi, NULL };
-	struct ahci_host_priv *hpriv;
-	struct ata_host *host;
-	struct resource *mem;
-	int irq;
-	int n_ports;
-	int i;
-	int rc;
-
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!mem) {
-		dev_err(dev, "no mmio space\n");
-		return -EINVAL;
-	}
-
-	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
-		dev_err(dev, "no irq\n");
-		return -EINVAL;
-	}
-
-	if (pdata && pdata->ata_port_info)
-		pi = *pdata->ata_port_info;
-
-	hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
-	if (!hpriv) {
-		dev_err(dev, "can't alloc ahci_host_priv\n");
-		return -ENOMEM;
-	}
-
-	hpriv->flags |= (unsigned long)pi.private_data;
-
-	hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
-	if (!hpriv->mmio) {
-		dev_err(dev, "can't map %pR\n", mem);
-		return -ENOMEM;
-	}
-
-	/*
-	 * Some platforms might need to prepare for mmio region access,
-	 * which could be done in the following init call. So, the mmio
-	 * region shouldn't be accessed before init (if provided) has
-	 * returned successfully.
-	 */
-	if (pdata && pdata->init) {
-		rc = pdata->init(dev, hpriv->mmio);
-		if (rc)
-			return rc;
-	}
-
-	ahci_save_initial_config(dev, hpriv,
-		pdata ? pdata->force_port_map : 0,
-		pdata ? pdata->mask_port_map  : 0);
-
-	/* prepare host */
-	if (hpriv->cap & HOST_CAP_NCQ)
-		pi.flags |= ATA_FLAG_NCQ;
-
-	if (hpriv->cap & HOST_CAP_PMP)
-		pi.flags |= ATA_FLAG_PMP;
-
-	ahci_set_em_messages(hpriv, &pi);
-
-	/* CAP.NP sometimes indicate the index of the last enabled
-	 * port, at other times, that of the last possible port, so
-	 * determining the maximum port number requires looking at
-	 * both CAP.NP and port_map.
-	 */
-	n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
-
-	host = ata_host_alloc_pinfo(dev, ppi, n_ports);
-	if (!host) {
-		rc = -ENOMEM;
-		goto err0;
-	}
-
-	host->private_data = hpriv;
-
-	if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
-		host->flags |= ATA_HOST_PARALLEL_SCAN;
-	else
-		printk(KERN_INFO "ahci: SSS flag set, parallel bus scan disabled\n");
-
-	if (pi.flags & ATA_FLAG_EM)
-		ahci_reset_em(host);
-
-	for (i = 0; i < host->n_ports; i++) {
-		struct ata_port *ap = host->ports[i];
-
-		ata_port_desc(ap, "mmio %pR", mem);
-		ata_port_desc(ap, "port 0x%x", 0x100 + ap->port_no * 0x80);
-
-		/* set enclosure management message type */
-		if (ap->flags & ATA_FLAG_EM)
-			ap->em_message_type = hpriv->em_msg_type;
-
-		/* disabled/not-implemented port */
-		if (!(hpriv->port_map & (1 << i)))
-			ap->ops = &ata_dummy_port_ops;
-	}
-
-	rc = ahci_reset_controller(host);
-	if (rc)
-		goto err0;
-
-	ahci_init_controller(host);
-	ahci_print_info(host, "platform");
-
-	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
-			       &ahci_platform_sht);
-	if (rc)
-		goto err0;
-
-	return 0;
-err0:
-	if (pdata && pdata->exit)
-		pdata->exit(dev);
-	return rc;
-}
-
-static int __devexit ahci_remove(struct platform_device *pdev)
-{
-	struct device *dev = &pdev->dev;
-	struct ahci_platform_data *pdata = dev->platform_data;
-	struct ata_host *host = dev_get_drvdata(dev);
-
-	ata_host_detach(host);
-
-	if (pdata && pdata->exit)
-		pdata->exit(dev);
-
-	return 0;
-}
-
-static struct platform_driver ahci_driver = {
-	.remove = __devexit_p(ahci_remove),
-	.driver = {
-		.name = "ahci",
-		.owner = THIS_MODULE,
-	},
-};
-
-static int __init ahci_init(void)
-{
-	return platform_driver_probe(&ahci_driver, ahci_probe);
-}
-module_init(ahci_init);
-
-static void __exit ahci_exit(void)
-{
-	platform_driver_unregister(&ahci_driver);
-}
-module_exit(ahci_exit);
-
-MODULE_DESCRIPTION("AHCI SATA platform driver");
-MODULE_AUTHOR("Anton Vorontsov <avorontsov@ru.mvista.com>");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:ahci");
diff --git a/drivers/ata/ahci_pltfm.c b/drivers/ata/ahci_pltfm.c
new file mode 100644
index 0000000..6fef1fa
--- /dev/null
+++ b/drivers/ata/ahci_pltfm.c
@@ -0,0 +1,197 @@
+/*
+ * AHCI SATA platform driver
+ *
+ * Copyright 2004-2005  Red Hat, Inc.
+ *   Jeff Garzik <jgarzik@pobox.com>
+ * Copyright 2010  MontaVista Software, LLC.
+ *   Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/gfp.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/libata.h>
+#include <linux/ahci_platform.h>
+#include "ahci.h"
+
+static struct scsi_host_template ahci_platform_sht = {
+	AHCI_SHT("ahci_platform"),
+};
+
+static int __init ahci_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ahci_platform_data *pdata = dev->platform_data;
+	struct ata_port_info pi = {
+		.flags		= AHCI_FLAG_COMMON,
+		.pio_mask	= ATA_PIO4,
+		.udma_mask	= ATA_UDMA6,
+		.port_ops	= &ahci_ops,
+	};
+	const struct ata_port_info *ppi[] = { &pi, NULL };
+	struct ahci_host_priv *hpriv;
+	struct ata_host *host;
+	struct resource *mem;
+	int irq;
+	int n_ports;
+	int i;
+	int rc;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem) {
+		dev_err(dev, "no mmio space\n");
+		return -EINVAL;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq <= 0) {
+		dev_err(dev, "no irq\n");
+		return -EINVAL;
+	}
+
+	if (pdata && pdata->ata_port_info)
+		pi = *pdata->ata_port_info;
+
+	hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
+	if (!hpriv) {
+		dev_err(dev, "can't alloc ahci_host_priv\n");
+		return -ENOMEM;
+	}
+
+	hpriv->flags |= (unsigned long)pi.private_data;
+
+	hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
+	if (!hpriv->mmio) {
+		dev_err(dev, "can't map %pR\n", mem);
+		return -ENOMEM;
+	}
+
+	/*
+	 * Some platforms might need to prepare for mmio region access,
+	 * which could be done in the following init call. So, the mmio
+	 * region shouldn't be accessed before init (if provided) has
+	 * returned successfully.
+	 */
+	if (pdata && pdata->init) {
+		rc = pdata->init(dev, hpriv->mmio);
+		if (rc)
+			return rc;
+	}
+
+	ahci_save_initial_config(dev, hpriv,
+		pdata ? pdata->force_port_map : 0,
+		pdata ? pdata->mask_port_map  : 0);
+
+	/* prepare host */
+	if (hpriv->cap & HOST_CAP_NCQ)
+		pi.flags |= ATA_FLAG_NCQ;
+
+	if (hpriv->cap & HOST_CAP_PMP)
+		pi.flags |= ATA_FLAG_PMP;
+
+	ahci_set_em_messages(hpriv, &pi);
+
+	/* CAP.NP sometimes indicate the index of the last enabled
+	 * port, at other times, that of the last possible port, so
+	 * determining the maximum port number requires looking at
+	 * both CAP.NP and port_map.
+	 */
+	n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
+
+	host = ata_host_alloc_pinfo(dev, ppi, n_ports);
+	if (!host) {
+		rc = -ENOMEM;
+		goto err0;
+	}
+
+	host->private_data = hpriv;
+
+	if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
+		host->flags |= ATA_HOST_PARALLEL_SCAN;
+	else
+		printk(KERN_INFO "ahci: SSS flag set, parallel bus scan disabled\n");
+
+	if (pi.flags & ATA_FLAG_EM)
+		ahci_reset_em(host);
+
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+
+		ata_port_desc(ap, "mmio %pR", mem);
+		ata_port_desc(ap, "port 0x%x", 0x100 + ap->port_no * 0x80);
+
+		/* set enclosure management message type */
+		if (ap->flags & ATA_FLAG_EM)
+			ap->em_message_type = hpriv->em_msg_type;
+
+		/* disabled/not-implemented port */
+		if (!(hpriv->port_map & (1 << i)))
+			ap->ops = &ata_dummy_port_ops;
+	}
+
+	rc = ahci_reset_controller(host);
+	if (rc)
+		goto err0;
+
+	ahci_init_controller(host);
+	ahci_print_info(host, "platform");
+
+	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
+			       &ahci_platform_sht);
+	if (rc)
+		goto err0;
+
+	return 0;
+err0:
+	if (pdata && pdata->exit)
+		pdata->exit(dev);
+	return rc;
+}
+
+static int __devexit ahci_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ahci_platform_data *pdata = dev->platform_data;
+	struct ata_host *host = dev_get_drvdata(dev);
+
+	ata_host_detach(host);
+
+	if (pdata && pdata->exit)
+		pdata->exit(dev);
+
+	return 0;
+}
+
+static struct platform_driver ahci_driver = {
+	.remove = __devexit_p(ahci_remove),
+	.driver = {
+		.name = "ahci",
+		.owner = THIS_MODULE,
+	},
+};
+
+static int __init ahci_init(void)
+{
+	return platform_driver_probe(&ahci_driver, ahci_probe);
+}
+module_init(ahci_init);
+
+static void __exit ahci_exit(void)
+{
+	platform_driver_unregister(&ahci_driver);
+}
+module_exit(ahci_exit);
+
+MODULE_DESCRIPTION("AHCI SATA platform driver");
+MODULE_AUTHOR("Anton Vorontsov <avorontsov@ru.mvista.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:ahci");
-- 
1.7.3

[PATCH v2 2/3] ahci_pltfm: switch to module device table matching

[mkl0301 at gmail.com]

From: Mac Lin <mkl0301@gmail.com>

Switch the driver to use module device table matching mechanism to add SoC-specific parts to the generic driver.

Signed-off-by: Mac Lin <mkl0301@gmail.com>
---
 drivers/ata/ahci_pltfm.c |   14 +++++++++++++-
 drivers/ata/ahci_pltfm.h |   17 +++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletions(-)
 create mode 100644 drivers/ata/ahci_pltfm.h

diff --git a/drivers/ata/ahci_pltfm.c b/drivers/ata/ahci_pltfm.c
index 6fef1fa..6579d55 100644
--- a/drivers/ata/ahci_pltfm.c
+++ b/drivers/ata/ahci_pltfm.c
@@ -19,9 +19,11 @@
 #include <linux/interrupt.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
 #include <linux/libata.h>
 #include <linux/ahci_platform.h>
 #include "ahci.h"
+#include "ahci_pltfm.h"
 
 static struct scsi_host_template ahci_platform_sht = {
 	AHCI_SHT("ahci_platform"),
@@ -29,6 +31,7 @@ static struct scsi_host_template ahci_platform_sht = {
 
 static int __init ahci_probe(struct platform_device *pdev)
 {
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
 	struct device *dev = &pdev->dev;
 	struct ahci_platform_data *pdata = dev->platform_data;
 	struct ata_port_info pi = {
@@ -46,6 +49,9 @@ static int __init ahci_probe(struct platform_device *pdev)
 	int i;
 	int rc;
 
+	if (!pdata && platid && platid->driver_data)
+		pdata = (void *)platid->driver_data;
+
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!mem) {
 		dev_err(dev, "no mmio space\n");
@@ -171,12 +177,19 @@ static int __devexit ahci_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct platform_device_id ahci_pltfm_ids[] = {
+	{ "ahci", },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, ahci_pltfm_ids);
+
 static struct platform_driver ahci_driver = {
 	.remove = __devexit_p(ahci_remove),
 	.driver = {
 		.name = "ahci",
 		.owner = THIS_MODULE,
 	},
+	.id_table	= ahci_pltfm_ids,
 };
 
 static int __init ahci_init(void)
@@ -194,4 +207,3 @@ module_exit(ahci_exit);
 MODULE_DESCRIPTION("AHCI SATA platform driver");
 MODULE_AUTHOR("Anton Vorontsov <avorontsov@ru.mvista.com>");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:ahci");
diff --git a/drivers/ata/ahci_pltfm.h b/drivers/ata/ahci_pltfm.h
new file mode 100644
index 0000000..b66390c
--- /dev/null
+++ b/drivers/ata/ahci_pltfm.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright 2010 MontaVista Software, LLC.
+ * Copyright 2010 Cavium Networks
+ *
+ * Authors: Anton Vorontsov <avorontsov@mvista.com>
+ *	    Mac Lin <mkl0301@gmail.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.
+ */
+
+#ifndef _DRIVERS_SATA_AHCI_PLTFM_H
+#define _DRIVERS_SATA_AHCI_PLTFM_H
+
+#endif /* _DRIVERS_SATA_AHCI_PLTFM_H */
+
-- 
1.7.3

[PATCH v2 3/3] ahci_platform: add support for CNS3xxx SoC devices

[mkl0301 at gmail.com]

From: Mac Lin <mkl0301@gmail.com>

CNS3xxx override the softreset function of ahci_platform ahci_softreset by
cns3xxx_ahci_softreset, which would retry ahci_do_softreset again with pmp=0 if
pmp=15 failed, for the controller has problem receiving D2H Reg FIS of the
different PMP setting of the previous sent H2D Reg FIS.

Following describe the isssue with original ahci_platform driver on
linux-2.6.37-rc3, arm/cns3xxx.

If CONFIG_SATA_PMP is enabled, while not using multiplier and connect the disks
directly to the board, the disk cannot be found due to software reset always
failed.

    ahci ahci.0: forcing PORTS_IMPL to 0x3
    ahci ahci.0: AHCI 0001.0100 32 slots 2 ports 3 Gbps 0x3 impl platform mode
    ahci ahci.0: flags: ncq sntf pm led clo only pmp pio slum part ccc
    scsi0 : ahci_platform
    scsi1 : ahci_platform
    ata1: SATA max UDMA/133 irq_stat 0x00400040, connection status changed irq 65
    ata2: SATA max UDMA/133 mmio [mem 0x83000000-0x83ffffff] port 0x180 irq 65
    ata2: SATA link down (SStatus 0 SControl 300)
    ata1: link is slow to respond, please be patient (ready=0)
    ata1: softreset failed (device not ready)
    ata1: link is slow to respond, please be patient (ready=0)
    ata1: softreset failed (device not ready)
    ata1: link is slow to respond, please be patient (ready=0)
    ata1: softreset failed (device not ready)
    ata1: limiting SATA link speed to 1.5 Gbps
    ata1: SATA link down (SStatus 1 SControl 310)

While using multiplier with CONFIG_SATA_PMP enabled, or using disks directly
without CONFIG_SATA_PMP have no issue. It seems the device is sending D2H Reg
FIS, but controller is not reflecting it on any known means.

Signed-off-by: Mac Lin <mkl0301@gmail.com>
---
 arch/arm/mach-cns3xxx/devices.c |    2 +-
 drivers/ata/Kconfig             |   11 +++++++
 drivers/ata/Makefile            |    1 +
 drivers/ata/ahci_cns3xxx.c      |   62 +++++++++++++++++++++++++++++++++++++++
 drivers/ata/ahci_pltfm.c        |    3 ++
 drivers/ata/ahci_pltfm.h        |    2 +
 6 files changed, 80 insertions(+), 1 deletions(-)
 create mode 100644 drivers/ata/ahci_cns3xxx.c

diff --git a/arch/arm/mach-cns3xxx/devices.c b/arch/arm/mach-cns3xxx/devices.c
index 50b4d31..b496f02 100644
--- a/arch/arm/mach-cns3xxx/devices.c
+++ b/arch/arm/mach-cns3xxx/devices.c
@@ -40,7 +40,7 @@ static struct resource cns3xxx_ahci_resource[] = {
 static u64 cns3xxx_ahci_dmamask = DMA_BIT_MASK(32);
 
 static struct platform_device cns3xxx_ahci_pdev = {
-	.name		= "ahci",
+	.name		= "ahci-cns3xxx",
 	.id		= 0,
 	.resource	= cns3xxx_ahci_resource,
 	.num_resources	= ARRAY_SIZE(cns3xxx_ahci_resource),
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 36e2319..5d8b1a3 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -75,6 +75,17 @@ config SATA_AHCI_PLATFORM
 
 	  If unsure, say N.
 
+config SATA_AHCI_CNS3XXX
+	bool "AHCI Support on the Cavium Networks CNS3xxx SOC"
+	depends on ARCH_CNS3XXX
+	depends on SATA_AHCI_PLATFORM
+	help
+	  This option enables AHCI platform driver to support CNS3xxx
+	  System-on-Chip devices. This is only needed when using CNS3xxx AHCI
+	  controller.
+
+	  If unsure, say N.
+
 config SATA_FSL
 	tristate "Freescale 3.0Gbps SATA support"
 	depends on FSL_SOC
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 5b62be8..a0745e5 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_PATA_LEGACY)	+= pata_legacy.o
 
 obj-$(CONFIG_SATA_AHCI_PLATFORM) 		+= ahci_platform.o libahci.o
 ahci_platform-y					:= ahci_pltfm.o
+ahci_platform-$(CONFIG_SATA_AHCI_CNS3XXX) 	+= ahci_cns3xxx.o
 
 libata-y	:= libata-core.o libata-scsi.o libata-eh.o libata-transport.o
 libata-$(CONFIG_ATA_SFF)	+= libata-sff.o
diff --git a/drivers/ata/ahci_cns3xxx.c b/drivers/ata/ahci_cns3xxx.c
new file mode 100644
index 0000000..f7a238e
--- /dev/null
+++ b/drivers/ata/ahci_cns3xxx.c
@@ -0,0 +1,62 @@
+/*
+ * AHCI support for CNS3xxx SoC
+ *
+ * Copyright 2010 MontaVista Software, LLC.
+ * Copyright 2010 Cavium Networks
+ *
+ * Authors: Anton Vorontsov <avorontsov@mvista.com>
+ *	    Mac Lin <mkl0301@gmail.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/libata.h>
+#include <linux/ahci_platform.h>
+#include "ahci.h"
+
+static int cns3xxx_ahci_softreset(struct ata_link *link, unsigned int *class,
+			  unsigned long deadline)
+{
+	int pmp = sata_srst_pmp(link);
+	int ret;
+	DPRINTK("ENTER\n");
+
+	ret = ahci_do_softreset(link, class, pmp, deadline, ahci_check_ready);
+	if (pmp && ret)
+		return ahci_do_softreset(link, class, 0, deadline,
+			ahci_check_ready);
+	else
+		return ret;
+}
+
+static int cns3xxx_ahci_init(struct device *dev, void __iomem *addr)
+{
+	/*
+	 * TODO: move cns3xxx_ahci_init to here after cns3xxx_pwr*() calls are
+	 * thread-safe
+	 */
+
+	return 0;
+}
+
+static struct ata_port_operations cns3xxx_ahci_ops = {
+	.inherits		= &ahci_ops,
+	.softreset		= cns3xxx_ahci_softreset,
+};
+
+static const struct ata_port_info cns3xxx_ata_port_info = {
+	.flags		= AHCI_FLAG_COMMON,
+	.pio_mask	= ATA_PIO4,
+	.udma_mask	= ATA_UDMA6,
+	.port_ops	= &cns3xxx_ahci_ops,
+};
+
+struct ahci_platform_data cns3xxx_ahci_platform_data = {
+	.init		= cns3xxx_ahci_init,
+	.ata_port_info	= &cns3xxx_ata_port_info,
+	.force_port_map = 0,
+	.mask_port_map = 0,
+};
+
diff --git a/drivers/ata/ahci_pltfm.c b/drivers/ata/ahci_pltfm.c
index 6579d55..03406f8 100644
--- a/drivers/ata/ahci_pltfm.c
+++ b/drivers/ata/ahci_pltfm.c
@@ -179,6 +179,9 @@ static int __devexit ahci_remove(struct platform_device *pdev)
 
 static const struct platform_device_id ahci_pltfm_ids[] = {
 	{ "ahci", },
+#ifdef CONFIG_SATA_AHCI_CNS3XXX
+	{ "ahci-cns3xxx", (kernel_ulong_t)&cns3xxx_ahci_platform_data},
+#endif
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, ahci_pltfm_ids);
diff --git a/drivers/ata/ahci_pltfm.h b/drivers/ata/ahci_pltfm.h
index b66390c..e07bf70 100644
--- a/drivers/ata/ahci_pltfm.h
+++ b/drivers/ata/ahci_pltfm.h
@@ -13,5 +13,7 @@
 #ifndef _DRIVERS_SATA_AHCI_PLTFM_H
 #define _DRIVERS_SATA_AHCI_PLTFM_H
 
+extern struct ahci_platform_data cns3xxx_ahci_platform_data;
+
 #endif /* _DRIVERS_SATA_AHCI_PLTFM_H */
 
-- 
1.7.3

[PATCH v2 0/3] add CNS3xxx AHCI support

[Jeff Garzik]

On 01/05/2011 12:43 AM, mkl0301 at gmail.com wrote:
> From: Mac Lin<mkl0301@gmail.com>
>
> v2:
>   - Switch ahci_platform to module device table matching to add SoC specific support
>
> v1: http://www.spinics.net/lists/arm-kernel/msg106236.html
>   - Add CNS3xxx SoC specific AHCI support
>
> This patchset is based on linux-2.6.37-rc2
>
> Mac Lin (3):
>        ahci_platform: rename to ahci_pltfm, but keep the original module name
>        ahci_pltfm: switch to module device table matching
>        ahci_platform: add support for CNS3xxx SoC devices
>
>   arch/arm/mach-cns3xxx/devices.c |    2 +-
>   drivers/ata/Kconfig             |   11 ++
>   drivers/ata/Makefile            |    5 +-
>   drivers/ata/ahci_cns3xxx.c      |   62 +++++++++++
>   drivers/ata/ahci_platform.c     |  197 ------------------------------------
>   drivers/ata/ahci_pltfm.c        |  212 +++++++++++++++++++++++++++++++++++++++
>   drivers/ata/ahci_pltfm.h        |   19 ++++
>   7 files changed, 309 insertions(+), 199 deletions(-)
>   create mode 100644 drivers/ata/ahci_cns3xxx.c
>   delete mode 100644 drivers/ata/ahci_platform.c
>   create mode 100644 drivers/ata/ahci_pltfm.c
>   create mode 100644 drivers/ata/ahci_pltfm.h

It is overkill to rename the entirety of ahci_platform just for one 
override function.

This sort of thing I would have expected to be added directly to 
ahci_platform.c.

	Jeff

[PATCH v2 0/3] add CNS3xxx AHCI support

[Lin Mac]

2011/1/6 Jeff Garzik <jgarzik@pobox.com>:
> On 01/05/2011 12:43 AM, mkl0301 at gmail.com wrote:
>>
>> From: Mac Lin<mkl0301@gmail.com>
>>
>> v2:
>> ?- Switch ahci_platform to module device table matching to add SoC
>> specific support
>>
>> v1: http://www.spinics.net/lists/arm-kernel/msg106236.html
>> ?- Add CNS3xxx SoC specific AHCI support
>>
>> This patchset is based on linux-2.6.37-rc2
>>
>> Mac Lin (3):
>> ? ? ? ahci_platform: rename to ahci_pltfm, but keep the original module
>> name
>> ? ? ? ahci_pltfm: switch to module device table matching
>> ? ? ? ahci_platform: add support for CNS3xxx SoC devices
>>
>> ?arch/arm/mach-cns3xxx/devices.c | ? ?2 +-
>> ?drivers/ata/Kconfig ? ? ? ? ? ? | ? 11 ++
>> ?drivers/ata/Makefile ? ? ? ? ? ?| ? ?5 +-
>> ?drivers/ata/ahci_cns3xxx.c ? ? ?| ? 62 +++++++++++
>> ?drivers/ata/ahci_platform.c ? ? | ?197
>> ------------------------------------
>> ?drivers/ata/ahci_pltfm.c ? ? ? ?| ?212
>> +++++++++++++++++++++++++++++++++++++++
>> ?drivers/ata/ahci_pltfm.h ? ? ? ?| ? 19 ++++
>> ?7 files changed, 309 insertions(+), 199 deletions(-)
>> ?create mode 100644 drivers/ata/ahci_cns3xxx.c
>> ?delete mode 100644 drivers/ata/ahci_platform.c
>> ?create mode 100644 drivers/ata/ahci_pltfm.c
>> ?create mode 100644 drivers/ata/ahci_pltfm.h
>
> It is overkill to rename the entirety of ahci_platform just for one override
> function.
> This sort of thing I would have expected to be added directly to
> ahci_platform.c.
It might be overkill for only one controller. but it is more clean and
readable to have different SoC specific changes in separate files,
especially when more SoCs need to make similar changes.

I will add them directly to ahci_platform.c if you insist.

Best Regards,
Mac Lin

[PATCH v2 0/3] add CNS3xxx AHCI support

[Anton Vorontsov]

On Thu, Jan 06, 2011 at 02:43:08PM +0800, Lin Mac wrote:
[...]
> > It is overkill to rename the entirety of ahci_platform just for one override
> > function.
> > This sort of thing I would have expected to be added directly to
> > ahci_platform.c.
> It might be overkill for only one controller. but it is more clean and
> readable to have different SoC specific changes in separate files,
> especially when more SoCs need to make similar changes.

I think that renaming the file is not necessary. You can just
rename the module in the makefile.

Personally I like the current approach more than putting
controller-specific fixups directly into ahci_platform.

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru at gmail.com

[PATCH v2 2/3] ahci_pltfm: switch to module device table matching

[Anton Vorontsov]

On Wed, Jan 05, 2011 at 01:43:06PM +0800, mkl0301 at gmail.com wrote:
> From: Mac Lin <mkl0301@gmail.com>
> 
> Switch the driver to use module device table matching mechanism to add SoC-specific parts to the generic driver.
> 
> Signed-off-by: Mac Lin <mkl0301@gmail.com>
> ---
>  drivers/ata/ahci_pltfm.c |   14 +++++++++++++-
>  drivers/ata/ahci_pltfm.h |   17 +++++++++++++++++
>  2 files changed, 30 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/ata/ahci_pltfm.h
> 
[..]
> + * 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.
> + */
> +
> +#ifndef _DRIVERS_SATA_AHCI_PLTFM_H
> +#define _DRIVERS_SATA_AHCI_PLTFM_H
> +
> +#endif /* _DRIVERS_SATA_AHCI_PLTFM_H */
> +
> --

No need for this empty line at the end of the file.

-- 
Anton Vorontsov
Email: cbouatmailru at gmail.com

[PATCH v2 3/3] ahci_platform: add support for CNS3xxx SoC devices

[Anton Vorontsov]

On Wed, Jan 05, 2011 at 01:43:07PM +0800, mkl0301 at gmail.com wrote:
> From: Mac Lin <mkl0301@gmail.com>
> 
> CNS3xxx override the softreset function of ahci_platform ahci_softreset by
> cns3xxx_ahci_softreset, which would retry ahci_do_softreset again with pmp=0 if
> pmp=15 failed, for the controller has problem receiving D2H Reg FIS of the
> different PMP setting of the previous sent H2D Reg FIS.
> 
> Following describe the isssue with original ahci_platform driver on
> linux-2.6.37-rc3, arm/cns3xxx.
> 
> If CONFIG_SATA_PMP is enabled, while not using multiplier and connect the disks
> directly to the board, the disk cannot be found due to software reset always
> failed.
> 
>     ahci ahci.0: forcing PORTS_IMPL to 0x3
>     ahci ahci.0: AHCI 0001.0100 32 slots 2 ports 3 Gbps 0x3 impl platform mode
>     ahci ahci.0: flags: ncq sntf pm led clo only pmp pio slum part ccc
>     scsi0 : ahci_platform
>     scsi1 : ahci_platform
>     ata1: SATA max UDMA/133 irq_stat 0x00400040, connection status changed irq 65
>     ata2: SATA max UDMA/133 mmio [mem 0x83000000-0x83ffffff] port 0x180 irq 65
>     ata2: SATA link down (SStatus 0 SControl 300)
>     ata1: link is slow to respond, please be patient (ready=0)
>     ata1: softreset failed (device not ready)
>     ata1: link is slow to respond, please be patient (ready=0)
>     ata1: softreset failed (device not ready)
>     ata1: link is slow to respond, please be patient (ready=0)
>     ata1: softreset failed (device not ready)
>     ata1: limiting SATA link speed to 1.5 Gbps
>     ata1: SATA link down (SStatus 1 SControl 310)
> 
> While using multiplier with CONFIG_SATA_PMP enabled, or using disks directly
> without CONFIG_SATA_PMP have no issue. It seems the device is sending D2H Reg
> FIS, but controller is not reflecting it on any known means.
> 
> Signed-off-by: Mac Lin <mkl0301@gmail.com>

I like this patch, thanks Mac! Few cosmetic comments below.

> ---
>  arch/arm/mach-cns3xxx/devices.c |    2 +-
>  drivers/ata/Kconfig             |   11 +++++++
>  drivers/ata/Makefile            |    1 +
>  drivers/ata/ahci_cns3xxx.c      |   62 +++++++++++++++++++++++++++++++++++++++
>  drivers/ata/ahci_pltfm.c        |    3 ++
>  drivers/ata/ahci_pltfm.h        |    2 +
>  6 files changed, 80 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/ata/ahci_cns3xxx.c
> 
> diff --git a/arch/arm/mach-cns3xxx/devices.c b/arch/arm/mach-cns3xxx/devices.c
> index 50b4d31..b496f02 100644
> --- a/arch/arm/mach-cns3xxx/devices.c
> +++ b/arch/arm/mach-cns3xxx/devices.c
> @@ -40,7 +40,7 @@ static struct resource cns3xxx_ahci_resource[] = {
>  static u64 cns3xxx_ahci_dmamask = DMA_BIT_MASK(32);
>  
>  static struct platform_device cns3xxx_ahci_pdev = {
> -	.name		= "ahci",
> +	.name		= "ahci-cns3xxx",
>  	.id		= 0,
>  	.resource	= cns3xxx_ahci_resource,
>  	.num_resources	= ARRAY_SIZE(cns3xxx_ahci_resource),
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 36e2319..5d8b1a3 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -75,6 +75,17 @@ config SATA_AHCI_PLATFORM
>  
>  	  If unsure, say N.
>  
> +config SATA_AHCI_CNS3XXX
> +	bool "AHCI Support on the Cavium Networks CNS3xxx SOC"
> +	depends on ARCH_CNS3XXX
> +	depends on SATA_AHCI_PLATFORM
> +	help
> +	  This option enables AHCI platform driver to support CNS3xxx
> +	  System-on-Chip devices. This is only needed when using CNS3xxx AHCI
> +	  controller.
> +
> +	  If unsure, say N.
> +
>  config SATA_FSL
>  	tristate "Freescale 3.0Gbps SATA support"
>  	depends on FSL_SOC
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 5b62be8..a0745e5 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -100,6 +100,7 @@ obj-$(CONFIG_PATA_LEGACY)	+= pata_legacy.o
>  
>  obj-$(CONFIG_SATA_AHCI_PLATFORM) 		+= ahci_platform.o libahci.o
>  ahci_platform-y					:= ahci_pltfm.o
> +ahci_platform-$(CONFIG_SATA_AHCI_CNS3XXX) 	+= ahci_cns3xxx.o
>  
>  libata-y	:= libata-core.o libata-scsi.o libata-eh.o libata-transport.o
>  libata-$(CONFIG_ATA_SFF)	+= libata-sff.o
> diff --git a/drivers/ata/ahci_cns3xxx.c b/drivers/ata/ahci_cns3xxx.c
> new file mode 100644
> index 0000000..f7a238e
> --- /dev/null
> +++ b/drivers/ata/ahci_cns3xxx.c
> @@ -0,0 +1,62 @@
> +/*
> + * AHCI support for CNS3xxx SoC
> + *
> + * Copyright 2010 MontaVista Software, LLC.
> + * Copyright 2010 Cavium Networks
> + *
> + * Authors: Anton Vorontsov <avorontsov@mvista.com>
> + *	    Mac Lin <mkl0301@gmail.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/libata.h>
> +#include <linux/ahci_platform.h>
> +#include "ahci.h"
> +
> +static int cns3xxx_ahci_softreset(struct ata_link *link, unsigned int *class,
> +			  unsigned long deadline)
> +{
> +	int pmp = sata_srst_pmp(link);
> +	int ret;
> +	DPRINTK("ENTER\n")

I'd remove this dprintk. Or at least, put an empty line after
'int ret;'

> +
> +	ret = ahci_do_softreset(link, class, pmp, deadline, ahci_check_ready);
> +	if (pmp && ret)
> +		return ahci_do_softreset(link, class, 0, deadline,
> +			ahci_check_ready);
> +	else
> +		return ret;

No need for 'else'

> +}
> +
> +static int cns3xxx_ahci_init(struct device *dev, void __iomem *addr)
> +{
> +	/*
> +	 * TODO: move cns3xxx_ahci_init to here after cns3xxx_pwr*() calls are
> +	 * thread-safe
> +	 */
> +
> +	return 0;
> +}

This is effectively no-op, just remove this function completely.
You may move the comment on top of the file, if you like.

> +static struct ata_port_operations cns3xxx_ahci_ops = {
> +	.inherits		= &ahci_ops,
> +	.softreset		= cns3xxx_ahci_softreset,
> +};
> +
> +static const struct ata_port_info cns3xxx_ata_port_info = {
> +	.flags		= AHCI_FLAG_COMMON,
> +	.pio_mask	= ATA_PIO4,
> +	.udma_mask	= ATA_UDMA6,
> +	.port_ops	= &cns3xxx_ahci_ops,
> +};
> +
> +struct ahci_platform_data cns3xxx_ahci_platform_data = {
> +	.init		= cns3xxx_ahci_init,
> +	.ata_port_info	= &cns3xxx_ata_port_info,
> +	.force_port_map = 0,
> +	.mask_port_map = 0,

You don't actually need to fill mask_port_map, as well as
force_port_map. cns3xxx_ahci_platform_data is global, thus will
initialize with zeroes.

> +};
> +

No need for this empty line (i.e. at the end of the file).

Thanks!

-- 
Anton Vorontsov
Email: cbouatmailru at gmail.com

[PATCH v2 0/3] add CNS3xxx AHCI support

[Lin Mac]

2011/1/6 Anton Vorontsov <cbouatmailru@gmail.com>:
> On Thu, Jan 06, 2011 at 02:43:08PM +0800, Lin Mac wrote:
> [...]
>> > It is overkill to rename the entirety of ahci_platform just for one override
>> > function.
>> > This sort of thing I would have expected to be added directly to
>> > ahci_platform.c.
>> It might be overkill for only one controller. but it is more clean and
>> readable to have different SoC specific changes in separate files,
>> especially when more SoCs need to make similar changes.
> I think that renaming the file is not necessary. You can just
> rename the module in the makefile.
That is for consist end user experience. I wouldn't like to find my
script not working anymore when changing to some linux version, simply
because someone fix some issue on some platform I'm not using.

> Personally I like the current approach more than putting
> controller-specific fixups directly into ahci_platform.

Best Regards,
Mac Lin.

[PATCH v2 0/3] add CNS3xxx AHCI support

[Jeff Garzik]

On 01/06/2011 05:51 AM, Anton Vorontsov wrote:
> On Thu, Jan 06, 2011 at 02:43:08PM +0800, Lin Mac wrote:
> [...]
>>> It is overkill to rename the entirety of ahci_platform just for one override
>>> function.
>>> This sort of thing I would have expected to be added directly to
>>> ahci_platform.c.
>> It might be overkill for only one controller. but it is more clean and
>> readable to have different SoC specific changes in separate files,
>> especially when more SoCs need to make similar changes.
>
> I think that renaming the file is not necessary. You can just
> rename the module in the makefile.
>
> Personally I like the current approach more than putting
> controller-specific fixups directly into ahci_platform.

My main objection is the renaming.  If ahci_platform wants to export 
some symbols for SoC modules like ahci_platform_cns, that's ok.

In general, follow the library approach rather than linking modules 
together in strange ways.

	Jeff

[PATCH v2 0/3] add CNS3xxx AHCI support

[Anton Vorontsov]

On Thu, Jan 06, 2011 at 06:17:56PM -0500, Jeff Garzik wrote:
> >[...]
> >>>It is overkill to rename the entirety of ahci_platform just for one override
> >>>function.
> >>>This sort of thing I would have expected to be added directly to
> >>>ahci_platform.c.
> >>It might be overkill for only one controller. but it is more clean and
> >>readable to have different SoC specific changes in separate files,
> >>especially when more SoCs need to make similar changes.
> >
> >I think that renaming the file is not necessary. You can just
> >rename the module in the makefile.
> >
> >Personally I like the current approach more than putting
> >controller-specific fixups directly into ahci_platform.
> 
> My main objection is the renaming.  If ahci_platform wants to export
> some symbols for SoC modules like ahci_platform_cns, that's ok.
> 
> In general, follow the library approach rather than linking modules
> together in strange ways.

In ahci_platform case there's almost nothing to factor out to a
library. Each platform just needs its own small "fixups" that we
normally pass via platform data.

We would happily pass them via platform data, and initially Mac
did exactly this, see:

http://ns3.spinics.net/lists/linux-ide/msg39554.html

You may notice that all platform-specific quirks are in the
platform code, which is neat. But there is a problem: once
the platform code needs to use libata, then we have to
build libata into the kernel (no modules possible), i.e.

http://ns3.spinics.net/lists/linux-ide/msg39656.html

It's not something new, we have the same "problem" with SDHCI,
USB and plenty other drivers. But the solution is simple, for
example see drivers/mmc/host/sdhci-pltfm.c and sdhci-cns3xxx.c
(and sdhci-esdhc-imx.c as another example of platform-specific
hooks for sdhci-pltfm).

Of course, we could make full-fledged platform driver just for
CNS3xxx, but that's kind of overkill. IMO, it's better to make
ahci_platform flexible and suitable for generic use.

As for renaming, I agree that ahci_platform.c file does not
need to be renamed, I still think that we could just rename
the module (so far it is used only on CNS3xxx platforms, so
no big deal even if anybody rely on the module name, which
I doubt.)

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru at gmail.com

[PATCH v2 0/3] add CNS3xxx AHCI support

[Lin Mac]

2011/1/7 Anton Vorontsov <cbouatmailru@gmail.com>:
> On Thu, Jan 06, 2011 at 06:17:56PM -0500, Jeff Garzik wrote:
>> >[...]
>> >>>It is overkill to rename the entirety of ahci_platform just for one override
>> >>>function.
>> >>>This sort of thing I would have expected to be added directly to
>> >>>ahci_platform.c.
>> >>It might be overkill for only one controller. but it is more clean and
>> >>readable to have different SoC specific changes in separate files,
>> >>especially when more SoCs need to make similar changes.
>> >
>> >I think that renaming the file is not necessary. You can just
>> >rename the module in the makefile.
>> >
>> >Personally I like the current approach more than putting
>> >controller-specific fixups directly into ahci_platform.
>>
>> My main objection is the renaming. ?If ahci_platform wants to export
>> some symbols for SoC modules like ahci_platform_cns, that's ok.
>>
>> In general, follow the library approach rather than linking modules
>> together in strange ways.
>
> In ahci_platform case there's almost nothing to factor out to a
> library. Each platform just needs its own small "fixups" that we
> normally pass via platform data.
>
> We would happily pass them via platform data, and initially Mac
> did exactly this, see:
>
> http://ns3.spinics.net/lists/linux-ide/msg39554.html
>
> You may notice that all platform-specific quirks are in the
> platform code, which is neat. But there is a problem: once
> the platform code needs to use libata, then we have to
> build libata into the kernel (no modules possible), i.e.
>
> http://ns3.spinics.net/lists/linux-ide/msg39656.html
>
> It's not something new, we have the same "problem" with SDHCI,
> USB and plenty other drivers. But the solution is simple, for
> example see drivers/mmc/host/sdhci-pltfm.c and sdhci-cns3xxx.c
> (and sdhci-esdhc-imx.c as another example of platform-specific
> hooks for sdhci-pltfm).
>
> Of course, we could make full-fledged platform driver just for
> CNS3xxx, but that's kind of overkill. IMO, it's better to make
> ahci_platform flexible and suitable for generic use.

Thanks for explaining all this.

> As for renaming, I agree that ahci_platform.c file does not
> need to be renamed, I still think that we could just rename
> the module (so far it is used only on CNS3xxx platforms, so
> no big deal even if anybody rely on the module name, which
> I doubt.)
Yes, you're right.
cns3xxx seems like the only platform that uses ahci_platform driver.
Keeping the same module name is not necessary.

We could rename the module ahci_platform to ahci_platforms(?), into
which link ahci_platform and SoC specific changes.
Is it acceptable?

Best Regards,
Mac Lin