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

login
register
mail settings
Submitter Lin Mac
Date Jan. 5, 2011, 5:43 a.m.
Message ID <1294206187-11487-4-git-send-email-mkl0301@gmail.com>
Download mbox | patch
Permalink /patch/77570/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Lin Mac - Jan. 5, 2011, 5:43 a.m.
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
Anton Vorontsov - Jan. 6, 2011, 11:02 a.m.
On Wed, Jan 05, 2011 at 01:43:07PM +0800, mkl0301@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!

Patch

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