[PATCH V6 1/5] AHCI Add the AHCI SATA feature on the MX53 platforms

[PATCH V6 1/5] AHCI Add the AHCI SATA feature on the MX53 platforms

[Richard Zhu]

Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
---
 arch/arm/mach-mx5/clock-mx51-mx53.c             |   19 ++++
 arch/arm/mach-mx5/devices-imx53.h               |    4 +
 arch/arm/plat-mxc/Makefile                      |    1 +
 arch/arm/plat-mxc/ahci_sata.c                   |  104 +++++++++++++++++++++++
 arch/arm/plat-mxc/devices/Kconfig               |    4 +
 arch/arm/plat-mxc/devices/Makefile              |    1 +
 arch/arm/plat-mxc/devices/platform-ahci-imx.c   |   57 ++++++++++++
 arch/arm/plat-mxc/include/mach/ahci_sata.h      |   33 +++++++
 arch/arm/plat-mxc/include/mach/devices-common.h |   10 ++
 9 files changed, 233 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-mxc/ahci_sata.c
 create mode 100644 arch/arm/plat-mxc/devices/platform-ahci-imx.c
 create mode 100644 arch/arm/plat-mxc/include/mach/ahci_sata.h

diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 7f20308..e1fadaf 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -1397,6 +1397,22 @@ static struct clk esdhc4_mx53_clk = {
 	.secondary = &esdhc4_ipg_clk,
 };
 
+static struct clk sata_clk = {
+	.parent = &ipg_clk,
+	.enable = _clk_max_enable,
+	.enable_reg = MXC_CCM_CCGR4,
+	.enable_shift = MXC_CCM_CCGRx_CG1_OFFSET,
+	.disable = _clk_max_disable,
+};
+
+static struct clk ahci_phy_clk = {
+	.parent = &usb_phy1_clk,
+};
+
+static struct clk ahci_dma_clk = {
+	.parent = &ahb_clk,
+};
+
 DEFINE_CLOCK(mipi_esc_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG5_OFFSET, NULL, NULL, NULL, &pll2_sw_clk);
 DEFINE_CLOCK(mipi_hsc2_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG4_OFFSET, NULL, NULL, &mipi_esc_clk, &pll2_sw_clk);
 DEFINE_CLOCK(mipi_hsc1_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG3_OFFSET, NULL, NULL, &mipi_hsc2_clk, &pll2_sw_clk);
@@ -1503,6 +1519,9 @@ static struct clk_lookup mx53_lookups[] = {
 	_REGISTER_CLOCK("imx-ssi.1", NULL, ssi2_clk)
 	_REGISTER_CLOCK("imx-ssi.2", NULL, ssi3_clk)
 	_REGISTER_CLOCK("imx-keypad", NULL, dummy_clk)
+	_REGISTER_CLOCK("imx53-ahci.0", "ahci", sata_clk)
+	_REGISTER_CLOCK("imx53-ahci.0", "ahci_phy", ahci_phy_clk)
+	_REGISTER_CLOCK("imx53-ahci.0", "ahci_dma", ahci_dma_clk)
 };
 
 static void clk_tree_init(void)
diff --git a/arch/arm/mach-mx5/devices-imx53.h b/arch/arm/mach-mx5/devices-imx53.h
index c27fe8b..bcb3af1 100644
--- a/arch/arm/mach-mx5/devices-imx53.h
+++ b/arch/arm/mach-mx5/devices-imx53.h
@@ -40,3 +40,7 @@ extern const struct imx_imx_ssi_data imx53_imx_ssi_data[];
 extern const struct imx_imx_keypad_data imx53_imx_keypad_data;
 #define imx53_add_imx_keypad(pdata)	\
 	imx_add_imx_keypad(&imx53_imx_keypad_data, pdata)
+
+extern const struct imx_ahci_imx_data imx53_ahci_imx_data __initconst;
+#define imx53_add_ahci_imx(id, pdata)   \
+	imx_add_ahci_imx(&imx53_ahci_imx_data, pdata)
diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
index d53c35f..cc8e65a 100644
--- a/arch/arm/plat-mxc/Makefile
+++ b/arch/arm/plat-mxc/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_ARCH_MXC_AUDMUX_V1) += audmux-v1.o
 obj-$(CONFIG_ARCH_MXC_AUDMUX_V2) += audmux-v2.o
 obj-$(CONFIG_MXC_DEBUG_BOARD) += 3ds_debugboard.o
 obj-$(CONFIG_CPU_FREQ_IMX)    += cpufreq.o
+obj-$(CONFIG_IMX_HAVE_PLATFORM_AHCI)	+= ahci_sata.o
 ifdef CONFIG_SND_IMX_SOC
 obj-y += ssi-fiq.o
 obj-y += ssi-fiq-ksym.o
diff --git a/arch/arm/plat-mxc/ahci_sata.c b/arch/arm/plat-mxc/ahci_sata.c
new file mode 100644
index 0000000..4f54816
--- /dev/null
+++ b/arch/arm/plat-mxc/ahci_sata.c
@@ -0,0 +1,104 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * 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 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <mach/ahci_sata.h>
+
+static struct clk *sata_clk, *sata_ref_clk;
+
+/* AHCI module Initialization, if return 0, initialization is successful. */
+int sata_init(struct device *dev, void __iomem *addr)
+{
+	u32 tmpdata;
+	int ret = 0;
+	struct clk *clk;
+
+	sata_clk = clk_get(dev, "ahci");
+	if (IS_ERR(sata_clk)) {
+		dev_err(dev, "no sata clock.\n");
+		return PTR_ERR(sata_clk);
+	}
+	ret = clk_enable(sata_clk);
+	if (ret) {
+		dev_err(dev, "can't enable sata clock.\n");
+		goto put_sata_clk;
+	}
+
+	/* FSL IMX AHCI SATA uses the internal usb phy1 clk on loco */
+	sata_ref_clk = clk_get(dev, "ahci_phy");
+	if (IS_ERR(sata_ref_clk)) {
+		dev_err(dev, "no sata ref clock.\n");
+		ret = PTR_ERR(sata_ref_clk);
+		goto release_sata_clk;
+	}
+	ret = clk_enable(sata_ref_clk);
+	if (ret) {
+		dev_err(dev, "can't enable sata ref clock.\n");
+		goto put_sata_ref_clk;
+	}
+
+	/* Get the AHB clock rate, and configure the TIMER1MS reg later */
+	clk = clk_get(dev, "ahci_dma");
+	if (IS_ERR(clk)) {
+		dev_err(dev, "no dma clock.\n");
+		ret = PTR_ERR(clk);
+		goto release_sata_ref_clk;
+	}
+	tmpdata = clk_get_rate(clk) / 1000;
+	clk_put(clk);
+
+	writel(tmpdata, addr + HOST_TIMER1MS);
+
+	tmpdata = readl(addr + HOST_CAP);
+	if (!(tmpdata & HOST_CAP_SSS)) {
+		tmpdata |= HOST_CAP_SSS;
+		writel(tmpdata, addr + HOST_CAP);
+	}
+
+	if (!(readl(addr + HOST_PORTS_IMPL) & 0x1))
+		writel((readl(addr + HOST_PORTS_IMPL) | 0x1),
+			addr + HOST_PORTS_IMPL);
+
+	return 0;
+
+release_sata_ref_clk:
+	clk_disable(sata_ref_clk);
+put_sata_ref_clk:
+	clk_put(sata_ref_clk);
+release_sata_clk:
+	clk_disable(sata_clk);
+put_sata_clk:
+	clk_put(sata_clk);
+
+	return ret;
+}
+
+void sata_exit(struct device *dev)
+{
+	clk_disable(sata_ref_clk);
+	clk_put(sata_ref_clk);
+
+	clk_disable(sata_clk);
+	clk_put(sata_clk);
+
+}
diff --git a/arch/arm/plat-mxc/devices/Kconfig b/arch/arm/plat-mxc/devices/Kconfig
index bd294ad..f63887b 100644
--- a/arch/arm/plat-mxc/devices/Kconfig
+++ b/arch/arm/plat-mxc/devices/Kconfig
@@ -76,3 +76,7 @@ config IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX
 
 config IMX_HAVE_PLATFORM_SPI_IMX
 	bool
+
+config IMX_HAVE_PLATFORM_AHCI
+	bool
+	default y if ARCH_MX53
diff --git a/arch/arm/plat-mxc/devices/Makefile b/arch/arm/plat-mxc/devices/Makefile
index b41bf97..e858ad9 100644
--- a/arch/arm/plat-mxc/devices/Makefile
+++ b/arch/arm/plat-mxc/devices/Makefile
@@ -25,3 +25,4 @@ obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_RTC) += platform-mxc_rtc.o
 obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_W1) += platform-mxc_w1.o
 obj-$(CONFIG_IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX) += platform-sdhci-esdhc-imx.o
 obj-$(CONFIG_IMX_HAVE_PLATFORM_SPI_IMX) +=  platform-spi_imx.o
+obj-$(CONFIG_IMX_HAVE_PLATFORM_AHCI) +=  platform-ahci-imx.o
diff --git a/arch/arm/plat-mxc/devices/platform-ahci-imx.c b/arch/arm/plat-mxc/devices/platform-ahci-imx.c
new file mode 100644
index 0000000..77c0552
--- /dev/null
+++ b/arch/arm/plat-mxc/devices/platform-ahci-imx.c
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * 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 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/dma-mapping.h>
+#include <asm/sizes.h>
+#include <mach/hardware.h>
+#include <mach/devices-common.h>
+
+#define imx_ahci_imx_data_entry_single(soc, _devid)		\
+	{								\
+		.devid = _devid,					\
+		.iobase = soc ## _SATA_BASE_ADDR,			\
+		.irq = soc ## _INT_SATA,				\
+	}
+
+#ifdef CONFIG_SOC_IMX53
+const struct imx_ahci_imx_data imx53_ahci_imx_data __initconst =
+	imx_ahci_imx_data_entry_single(MX53, "imx53-ahci");
+#endif
+
+struct platform_device *__init imx_add_ahci_imx(
+		const struct imx_ahci_imx_data *data,
+		const struct ahci_platform_data *pdata)
+{
+	struct resource res[] = {
+		{
+			.start = data->iobase,
+			.end = data->iobase + SZ_4K - 1,
+			.flags = IORESOURCE_MEM,
+		}, {
+			.start = data->irq,
+			.end = data->irq,
+			.flags = IORESOURCE_IRQ,
+		},
+	};
+
+	return imx_add_platform_device_dmamask(data->devid, 0,
+			res, ARRAY_SIZE(res),
+			pdata, sizeof(*pdata),  DMA_BIT_MASK(32));
+}
diff --git a/arch/arm/plat-mxc/include/mach/ahci_sata.h b/arch/arm/plat-mxc/include/mach/ahci_sata.h
new file mode 100644
index 0000000..ba297e1
--- /dev/null
+++ b/arch/arm/plat-mxc/include/mach/ahci_sata.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * 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 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef __PLAT_MXC_AHCI_SATA_H__
+#define __PLAT_MXC_AHCI_SATA_H__
+
+enum {
+	HOST_CAP = 0x00,
+	HOST_CAP_SSS = (1 << 27), /* Staggered Spin-up */
+	HOST_PORTS_IMPL	= 0x0c,
+	HOST_TIMER1MS = 0xe0, /* Timer 1-ms */
+};
+
+extern int sata_init(struct device *dev, void __iomem *addr);
+extern void sata_exit(struct device *dev);
+#endif /* __PLAT_MXC_AHCI_SATA_H__ */
diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h b/arch/arm/plat-mxc/include/mach/devices-common.h
index 524538a..f04e063 100644
--- a/arch/arm/plat-mxc/include/mach/devices-common.h
+++ b/arch/arm/plat-mxc/include/mach/devices-common.h
@@ -301,3 +301,13 @@ struct platform_device *__init imx_add_spi_imx(
 struct platform_device *imx_add_imx_dma(void);
 struct platform_device *imx_add_imx_sdma(char *name,
 	resource_size_t iobase, int irq, struct sdma_platform_data *pdata);
+
+#include <linux/ahci_platform.h>
+struct imx_ahci_imx_data {
+	const char *devid;
+	resource_size_t iobase;
+	resource_size_t irq;
+};
+struct platform_device *__init imx_add_ahci_imx(
+		const struct imx_ahci_imx_data *data,
+		const struct ahci_platform_data *pdata);
-- 
1.7.1

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Richard Zhu]

On imx53 AHCI, soft reset fails with IPMS set when PMP
is enabled but SATA HDD/ODD is connected to SATA port,
do soft reset again to port 0.
So the 'ahci_pmp_retry_srst_ops' is required when imx53
ahci is present.

Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
---
 drivers/ata/ahci_platform.c |   44 +++++++++++++++++++++++++++++++++++++-----
 1 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 6fef1fa..f32c91e 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -23,6 +23,41 @@
 #include <linux/ahci_platform.h>
 #include "ahci.h"
 
+enum ahci_type {
+	AHCI,		/* standard platform ahci */
+	IMX53_AHCI,	/* ahci on i.mx53 */
+};
+
+static struct platform_device_id ahci_devtype[] = {
+	{
+		.name = "ahci",
+		.driver_data = AHCI,
+	}, {
+		.name = "imx53-ahci",
+		.driver_data = IMX53_AHCI,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(platform, ahci_devtype);
+
+
+static const struct ata_port_info ahci_port_info[] = {
+	/* by features */
+	[AHCI] = {
+		.flags		= AHCI_FLAG_COMMON,
+		.pio_mask	= ATA_PIO4,
+		.udma_mask	= ATA_UDMA6,
+		.port_ops	= &ahci_ops,
+	},
+	[IMX53_AHCI] = {
+		.flags		= AHCI_FLAG_COMMON,
+		.pio_mask	= ATA_PIO4,
+		.udma_mask	= ATA_UDMA6,
+		.port_ops	= &ahci_pmp_retry_srst_ops,
+	},
+};
+
 static struct scsi_host_template ahci_platform_sht = {
 	AHCI_SHT("ahci_platform"),
 };
@@ -31,12 +66,8 @@ 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,
-	};
+	struct platform_device_id *id_entry = platform_get_device_id(pdev);
+	struct ata_port_info pi = ahci_port_info[id_entry->driver_data];
 	const struct ata_port_info *ppi[] = { &pi, NULL };
 	struct ahci_host_priv *hpriv;
 	struct ata_host *host;
@@ -177,6 +208,7 @@ static struct platform_driver ahci_driver = {
 		.name = "ahci",
 		.owner = THIS_MODULE,
 	},
+	.id_table	= ahci_devtype,
 };
 
 static int __init ahci_init(void)
-- 
1.7.1

[PATCH V6 3/5] MX53 Enable the AHCI SATA on MX53 ARD board

[Richard Zhu]

Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
---
 arch/arm/mach-mx5/board-mx53_ard.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/board-mx53_ard.c b/arch/arm/mach-mx5/board-mx53_ard.c
index 76a67c4..d265cee 100644
--- a/arch/arm/mach-mx5/board-mx53_ard.c
+++ b/arch/arm/mach-mx5/board-mx53_ard.c
@@ -27,6 +27,7 @@
 #include <mach/common.h>
 #include <mach/hardware.h>
 #include <mach/iomux-mx53.h>
+#include <mach/ahci_sata.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -220,6 +221,11 @@ static struct platform_device *devices[] __initdata = {
 	&ard_smsc_lan9220_device,
 };
 
+static struct ahci_platform_data sata_data = {
+	.init = sata_init,
+	.exit = sata_exit,
+};
+
 static void __init mx53_ard_board_init(void)
 {
 	imx53_soc_init();
@@ -234,6 +240,7 @@ static void __init mx53_ard_board_init(void)
 	imx53_add_imx_i2c(1, &mx53_ard_i2c2_data);
 	imx53_add_imx_i2c(2, &mx53_ard_i2c3_data);
 	imx_add_gpio_keys(&ard_button_data);
+	imx53_add_ahci_imx(0, &sata_data);
 }
 
 static void __init mx53_ard_timer_init(void)
-- 
1.7.1

[PATCH V6 4/5] MX53 Enable the AHCI SATA on MX53 LOCO board

[Richard Zhu]

Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
---
 arch/arm/mach-mx5/board-mx53_loco.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
index 4e1d51d..551fc7c 100644
--- a/arch/arm/mach-mx5/board-mx53_loco.c
+++ b/arch/arm/mach-mx5/board-mx53_loco.c
@@ -26,6 +26,7 @@
 #include <mach/common.h>
 #include <mach/hardware.h>
 #include <mach/iomux-mx53.h>
+#include <mach/ahci_sata.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -257,6 +258,11 @@ static const struct gpio_led_platform_data mx53loco_leds_data __initconst = {
 	.num_leds	= ARRAY_SIZE(mx53loco_leds),
 };
 
+static struct ahci_platform_data sata_data = {
+	.init = sata_init,
+	.exit = sata_exit,
+};
+
 static void __init mx53_loco_board_init(void)
 {
 	imx53_soc_init();
@@ -273,6 +279,7 @@ static void __init mx53_loco_board_init(void)
 	imx53_add_sdhci_esdhc_imx(2, &mx53_loco_sd3_data);
 	imx_add_gpio_keys(&loco_button_data);
 	gpio_led_register_device(-1, &mx53loco_leds_data);
+	imx53_add_ahci_imx(0, &sata_data);
 }
 
 static void __init mx53_loco_timer_init(void)
-- 
1.7.1

[PATCH V6 5/5] MX53 Enable the AHCI SATA on MX53 SMD board

[Richard Zhu]

Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
---
 arch/arm/mach-mx5/board-mx53_smd.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/board-mx53_smd.c b/arch/arm/mach-mx5/board-mx53_smd.c
index bc02894..1cea06b 100644
--- a/arch/arm/mach-mx5/board-mx53_smd.c
+++ b/arch/arm/mach-mx5/board-mx53_smd.c
@@ -26,6 +26,7 @@
 #include <mach/common.h>
 #include <mach/hardware.h>
 #include <mach/iomux-mx53.h>
+#include <mach/ahci_sata.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -111,6 +112,11 @@ static const struct imxi2c_platform_data mx53_smd_i2c_data __initconst = {
 	.bitrate = 100000,
 };
 
+static struct ahci_platform_data sata_data = {
+	.init = sata_init,
+	.exit = sata_exit,
+};
+
 static void __init mx53_smd_board_init(void)
 {
 	imx53_soc_init();
@@ -125,6 +131,7 @@ static void __init mx53_smd_board_init(void)
 	imx53_add_sdhci_esdhc_imx(0, NULL);
 	imx53_add_sdhci_esdhc_imx(1, NULL);
 	imx53_add_sdhci_esdhc_imx(2, NULL);
+	imx53_add_ahci_imx(0, &sata_data);
 }
 
 static void __init mx53_smd_timer_init(void)
-- 
1.7.1

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Eric Miao]

On Mon, Aug 29, 2011 at 3:18 PM, Richard Zhu <richard.zhu@linaro.org> wrote:
> On imx53 AHCI, soft reset fails with IPMS set when PMP
> is enabled but SATA HDD/ODD is connected to SATA port,
> do soft reset again to port 0.
> So the 'ahci_pmp_retry_srst_ops' is required when imx53
> ahci is present.
>
> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>

Looks good to me, I like this way very much.

Acked-by: Eric Miao <eric.miao@linaro.org>


> ---
> ?drivers/ata/ahci_platform.c | ? 44 +++++++++++++++++++++++++++++++++++++-----
> ?1 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index 6fef1fa..f32c91e 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -23,6 +23,41 @@
> ?#include <linux/ahci_platform.h>
> ?#include "ahci.h"
>
> +enum ahci_type {
> + ? ? ? AHCI, ? ? ? ? ? /* standard platform ahci */
> + ? ? ? IMX53_AHCI, ? ? /* ahci on i.mx53 */
> +};
> +
> +static struct platform_device_id ahci_devtype[] = {
> + ? ? ? {
> + ? ? ? ? ? ? ? .name = "ahci",
> + ? ? ? ? ? ? ? .driver_data = AHCI,
> + ? ? ? }, {
> + ? ? ? ? ? ? ? .name = "imx53-ahci",
> + ? ? ? ? ? ? ? .driver_data = IMX53_AHCI,
> + ? ? ? }, {
> + ? ? ? ? ? ? ? /* sentinel */
> + ? ? ? }
> +};
> +MODULE_DEVICE_TABLE(platform, ahci_devtype);
> +
> +
> +static const struct ata_port_info ahci_port_info[] = {
> + ? ? ? /* by features */
> + ? ? ? [AHCI] = {
> + ? ? ? ? ? ? ? .flags ? ? ? ? ?= AHCI_FLAG_COMMON,
> + ? ? ? ? ? ? ? .pio_mask ? ? ? = ATA_PIO4,
> + ? ? ? ? ? ? ? .udma_mask ? ? ?= ATA_UDMA6,
> + ? ? ? ? ? ? ? .port_ops ? ? ? = &ahci_ops,
> + ? ? ? },
> + ? ? ? [IMX53_AHCI] = {
> + ? ? ? ? ? ? ? .flags ? ? ? ? ?= AHCI_FLAG_COMMON,
> + ? ? ? ? ? ? ? .pio_mask ? ? ? = ATA_PIO4,
> + ? ? ? ? ? ? ? .udma_mask ? ? ?= ATA_UDMA6,
> + ? ? ? ? ? ? ? .port_ops ? ? ? = &ahci_pmp_retry_srst_ops,
> + ? ? ? },
> +};
> +
> ?static struct scsi_host_template ahci_platform_sht = {
> ? ? ? ?AHCI_SHT("ahci_platform"),
> ?};
> @@ -31,12 +66,8 @@ 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,
> - ? ? ? };
> + ? ? ? struct platform_device_id *id_entry = platform_get_device_id(pdev);
> + ? ? ? struct ata_port_info pi = ahci_port_info[id_entry->driver_data];
> ? ? ? ?const struct ata_port_info *ppi[] = { &pi, NULL };
> ? ? ? ?struct ahci_host_priv *hpriv;
> ? ? ? ?struct ata_host *host;
> @@ -177,6 +208,7 @@ static struct platform_driver ahci_driver = {
> ? ? ? ? ? ? ? ?.name = "ahci",
> ? ? ? ? ? ? ? ?.owner = THIS_MODULE,
> ? ? ? ?},
> + ? ? ? .id_table ? ? ? = ahci_devtype,
> ?};
>
> ?static int __init ahci_init(void)
> --
> 1.7.1
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Wolfram Sang]

On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote:
> On imx53 AHCI, soft reset fails with IPMS set when PMP
> is enabled but SATA HDD/ODD is connected to SATA port,
> do soft reset again to port 0.
> So the 'ahci_pmp_retry_srst_ops' is required when imx53
> ahci is present.
> 
> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
> ---
>  drivers/ata/ahci_platform.c |   44 +++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index 6fef1fa..f32c91e 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -23,6 +23,41 @@
>  #include <linux/ahci_platform.h>
>  #include "ahci.h"
>  
> +enum ahci_type {
> +	AHCI,		/* standard platform ahci */
> +	IMX53_AHCI,	/* ahci on i.mx53 */

How about making all the IMX*-naming more generic because other SoC might need
this somewhen, too?

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110829/c99bf92a/attachment.sig>

[PATCH V6 3/5] MX53 Enable the AHCI SATA on MX53 ARD board

[Eric Miao]

On Mon, Aug 29, 2011 at 3:18 PM, Richard Zhu <richard.zhu@linaro.org> wrote:
> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
> ---
> ?arch/arm/mach-mx5/board-mx53_ard.c | ? ?7 +++++++
> ?1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-mx5/board-mx53_ard.c b/arch/arm/mach-mx5/board-mx53_ard.c
> index 76a67c4..d265cee 100644
> --- a/arch/arm/mach-mx5/board-mx53_ard.c
> +++ b/arch/arm/mach-mx5/board-mx53_ard.c
> @@ -27,6 +27,7 @@
> ?#include <mach/common.h>
> ?#include <mach/hardware.h>
> ?#include <mach/iomux-mx53.h>
> +#include <mach/ahci_sata.h>
>
> ?#include <asm/mach-types.h>
> ?#include <asm/mach/arch.h>
> @@ -220,6 +221,11 @@ static struct platform_device *devices[] __initdata = {
> ? ? ? ?&ard_smsc_lan9220_device,
> ?};
>
> +static struct ahci_platform_data sata_data = {
> + ? ? ? .init = sata_init,
> + ? ? ? .exit = sata_exit,
> +};
> +

As this platform data is same across most of these boards, I'd suggest we
keep a common/default platform_data in

arch/arm/plat-mxc/devices/platform-imx-ahci.c

And pass NULL in these board files, making use of the above default one.
For those boards with exceptions (e.g. special operations like GPIO to
enable/disable something at init/exit), one can still have platform data
and pass the pointer.

> ?static void __init mx53_ard_board_init(void)
> ?{
> ? ? ? ?imx53_soc_init();
> @@ -234,6 +240,7 @@ static void __init mx53_ard_board_init(void)
> ? ? ? ?imx53_add_imx_i2c(1, &mx53_ard_i2c2_data);
> ? ? ? ?imx53_add_imx_i2c(2, &mx53_ard_i2c3_data);
> ? ? ? ?imx_add_gpio_keys(&ard_button_data);
> + ? ? ? imx53_add_ahci_imx(0, &sata_data);
> ?}
>
> ?static void __init mx53_ard_timer_init(void)
> --
> 1.7.1
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Eric Miao]

On Mon, Aug 29, 2011 at 4:36 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote:
>> On imx53 AHCI, soft reset fails with IPMS set when PMP
>> is enabled but SATA HDD/ODD is connected to SATA port,
>> do soft reset again to port 0.
>> So the 'ahci_pmp_retry_srst_ops' is required when imx53
>> ahci is present.
>>
>> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
>> ---
>> ?drivers/ata/ahci_platform.c | ? 44 +++++++++++++++++++++++++++++++++++++-----
>> ?1 files changed, 38 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
>> index 6fef1fa..f32c91e 100644
>> --- a/drivers/ata/ahci_platform.c
>> +++ b/drivers/ata/ahci_platform.c
>> @@ -23,6 +23,41 @@
>> ?#include <linux/ahci_platform.h>
>> ?#include "ahci.h"
>>
>> +enum ahci_type {
>> + ? ? AHCI, ? ? ? ? ? /* standard platform ahci */
>> + ? ? IMX53_AHCI, ? ? /* ahci on i.mx53 */
>
> How about making all the IMX*-naming more generic because other SoC might need
> this somewhen, too?

[PATCH V6 3/5] MX53 Enable the AHCI SATA on MX53 ARD board

[Richard Zhu]

Hi Eric:
I agree with your suggestion.
One more exception about ahci on imx53 smd board.
one sata_pwr_en is used to control the pwr supply on smd board.
It's better to enable this pwer_en obviously although that this pin is
enabled reserved.

the ahci driver use the default sata platform data on ard and loco boards, and
 the smd platform data on smd board refer to the sata_pwr_en usage.

Best Regards
Richard Zhu

On 29 August 2011 16:47, Eric Miao <eric.miao@linaro.org> wrote:
> On Mon, Aug 29, 2011 at 3:18 PM, Richard Zhu <richard.zhu@linaro.org> wrote:
>> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
>> ---
>> ?arch/arm/mach-mx5/board-mx53_ard.c | ? ?7 +++++++
>> ?1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-mx5/board-mx53_ard.c b/arch/arm/mach-mx5/board-mx53_ard.c
>> index 76a67c4..d265cee 100644
>> --- a/arch/arm/mach-mx5/board-mx53_ard.c
>> +++ b/arch/arm/mach-mx5/board-mx53_ard.c
>> @@ -27,6 +27,7 @@
>> ?#include <mach/common.h>
>> ?#include <mach/hardware.h>
>> ?#include <mach/iomux-mx53.h>
>> +#include <mach/ahci_sata.h>
>>
>> ?#include <asm/mach-types.h>
>> ?#include <asm/mach/arch.h>
>> @@ -220,6 +221,11 @@ static struct platform_device *devices[] __initdata = {
>> ? ? ? ?&ard_smsc_lan9220_device,
>> ?};
>>
>> +static struct ahci_platform_data sata_data = {
>> + ? ? ? .init = sata_init,
>> + ? ? ? .exit = sata_exit,
>> +};
>> +
>
> As this platform data is same across most of these boards, I'd suggest we
> keep a common/default platform_data in
>
> arch/arm/plat-mxc/devices/platform-imx-ahci.c
>
> And pass NULL in these board files, making use of the above default one.
> For those boards with exceptions (e.g. special operations like GPIO to
> enable/disable something at init/exit), one can still have platform data
> and pass the pointer.
>
>> ?static void __init mx53_ard_board_init(void)
>> ?{
>> ? ? ? ?imx53_soc_init();
>> @@ -234,6 +240,7 @@ static void __init mx53_ard_board_init(void)
>> ? ? ? ?imx53_add_imx_i2c(1, &mx53_ard_i2c2_data);
>> ? ? ? ?imx53_add_imx_i2c(2, &mx53_ard_i2c3_data);
>> ? ? ? ?imx_add_gpio_keys(&ard_button_data);
>> + ? ? ? imx53_add_ahci_imx(0, &sata_data);
>> ?}
>>
>> ?static void __init mx53_ard_timer_init(void)
>> --
>> 1.7.1
>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Wolfram Sang]

On Mon, Aug 29, 2011 at 04:51:46PM +0800, Eric Miao wrote:
> On Mon, Aug 29, 2011 at 4:36 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> > On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote:
> >> On imx53 AHCI, soft reset fails with IPMS set when PMP
> >> is enabled but SATA HDD/ODD is connected to SATA port,
> >> do soft reset again to port 0.
> >> So the 'ahci_pmp_retry_srst_ops' is required when imx53
> >> ahci is present.
> >>
> >> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
> >> ---
> >> ?drivers/ata/ahci_platform.c | ? 44 +++++++++++++++++++++++++++++++++++++-----
> >> ?1 files changed, 38 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> >> index 6fef1fa..f32c91e 100644
> >> --- a/drivers/ata/ahci_platform.c
> >> +++ b/drivers/ata/ahci_platform.c
> >> @@ -23,6 +23,41 @@
> >> ?#include <linux/ahci_platform.h>
> >> ?#include "ahci.h"
> >>
> >> +enum ahci_type {
> >> + ? ? AHCI, ? ? ? ? ? /* standard platform ahci */
> >> + ? ? IMX53_AHCI, ? ? /* ahci on i.mx53 */
> >
> > How about making all the IMX*-naming more generic because other SoC might need
> > this somewhen, too?
> 
> From a practical point of view, we can start with what we know already.
> As the SATA controller on "other SoC" so far, we don't know yet if they
> show the same issue (which we have to use ahci_pmp_retry_srst_ops).
> 
> So Wolfram, how about we consider a generic one once more controllers
> are being added?

If we do so later, and we then also change the platform_device_id to something
generic, we then also have to change all users, too. Is it so bad to change
IMX53_AHCI (and imx53-ahci and alike) to something like "ahci-pmp-retry-srst"
(or similar) now?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110829/16cc5b08/attachment.sig>

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Eric Miao]

On Mon, Aug 29, 2011 at 5:37 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Mon, Aug 29, 2011 at 04:51:46PM +0800, Eric Miao wrote:
>> On Mon, Aug 29, 2011 at 4:36 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> > On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote:
>> >> On imx53 AHCI, soft reset fails with IPMS set when PMP
>> >> is enabled but SATA HDD/ODD is connected to SATA port,
>> >> do soft reset again to port 0.
>> >> So the 'ahci_pmp_retry_srst_ops' is required when imx53
>> >> ahci is present.
>> >>
>> >> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
>> >> ---
>> >> ?drivers/ata/ahci_platform.c | ? 44 +++++++++++++++++++++++++++++++++++++-----
>> >> ?1 files changed, 38 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
>> >> index 6fef1fa..f32c91e 100644
>> >> --- a/drivers/ata/ahci_platform.c
>> >> +++ b/drivers/ata/ahci_platform.c
>> >> @@ -23,6 +23,41 @@
>> >> ?#include <linux/ahci_platform.h>
>> >> ?#include "ahci.h"
>> >>
>> >> +enum ahci_type {
>> >> + ? ? AHCI, ? ? ? ? ? /* standard platform ahci */
>> >> + ? ? IMX53_AHCI, ? ? /* ahci on i.mx53 */
>> >
>> > How about making all the IMX*-naming more generic because other SoC might need
>> > this somewhen, too?
>>
>> From a practical point of view, we can start with what we know already.
>> As the SATA controller on "other SoC" so far, we don't know yet if they
>> show the same issue (which we have to use ahci_pmp_retry_srst_ops).
>>
>> So Wolfram, how about we consider a generic one once more controllers
>> are being added?
>
> If we do so later, and we then also change the platform_device_id to something
> generic, we then also have to change all users, too. Is it so bad to change
> IMX53_AHCI (and imx53-ahci and alike) to something like "ahci-pmp-retry-srst"
> (or similar) now?

There are other fields in ata_port_info which might be different between
boards. Using a SoC specific name as an index to an array of ata_port_info
will be more flexible, as is adopted in drivers/ata/ahci.c - the PCI driver.

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Wolfram Sang]

On Mon, Aug 29, 2011 at 05:54:23PM +0800, Eric Miao wrote:
> On Mon, Aug 29, 2011 at 5:37 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> > On Mon, Aug 29, 2011 at 04:51:46PM +0800, Eric Miao wrote:
> >> On Mon, Aug 29, 2011 at 4:36 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> >> > On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote:
> >> >> On imx53 AHCI, soft reset fails with IPMS set when PMP
> >> >> is enabled but SATA HDD/ODD is connected to SATA port,
> >> >> do soft reset again to port 0.
> >> >> So the 'ahci_pmp_retry_srst_ops' is required when imx53
> >> >> ahci is present.
> >> >>
> >> >> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
> >> >> ---
> >> >> ?drivers/ata/ahci_platform.c | ? 44 +++++++++++++++++++++++++++++++++++++-----
> >> >> ?1 files changed, 38 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> >> >> index 6fef1fa..f32c91e 100644
> >> >> --- a/drivers/ata/ahci_platform.c
> >> >> +++ b/drivers/ata/ahci_platform.c
> >> >> @@ -23,6 +23,41 @@
> >> >> ?#include <linux/ahci_platform.h>
> >> >> ?#include "ahci.h"
> >> >>
> >> >> +enum ahci_type {
> >> >> + ? ? AHCI, ? ? ? ? ? /* standard platform ahci */
> >> >> + ? ? IMX53_AHCI, ? ? /* ahci on i.mx53 */
> >> >
> >> > How about making all the IMX*-naming more generic because other SoC might need
> >> > this somewhen, too?
> >>
> >> From a practical point of view, we can start with what we know already.
> >> As the SATA controller on "other SoC" so far, we don't know yet if they
> >> show the same issue (which we have to use ahci_pmp_retry_srst_ops).
> >>
> >> So Wolfram, how about we consider a generic one once more controllers
> >> are being added?
> >
> > If we do so later, and we then also change the platform_device_id to something
> > generic, we then also have to change all users, too. Is it so bad to change
> > IMX53_AHCI (and imx53-ahci and alike) to something like "ahci-pmp-retry-srst"
> > (or similar) now?
> 
> There are other fields in ata_port_info which might be different between
> boards. Using a SoC specific name as an index to an array of ata_port_info
> will be more flexible, as is adopted in drivers/ata/ahci.c - the PCI driver.

Well, that file also has generic fixups:

       [board_ahci_ign_iferr] =
...

which is what I had in mind thinking that the flaw was not too imx53 specific.
If you think I am wrong, then be it. I mainly wanted to avoid the fixups which
might be needed later.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110829/a13976b0/attachment.sig>

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Anton Vorontsov]

Hello,

On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote:
> On imx53 AHCI, soft reset fails with IPMS set when PMP
> is enabled but SATA HDD/ODD is connected to SATA port,
> do soft reset again to port 0.
> So the 'ahci_pmp_retry_srst_ops' is required when imx53
> ahci is present.
> 
> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
> ---
[...]
>  	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,
> -	};
> +	struct platform_device_id *id_entry = platform_get_device_id(pdev);
> +	struct ata_port_info pi = ahci_port_info[id_entry->driver_data];

Why not pass port info via platform_data? It seems to be platform
specific nowadays, so leave the default as is, but let the platforms
pass their own port info through platform_data.

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru at gmail.com

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Eric Miao]

On Mon, Aug 29, 2011 at 8:12 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
> Hello,
>
> On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote:
>> On imx53 AHCI, soft reset fails with IPMS set when PMP
>> is enabled but SATA HDD/ODD is connected to SATA port,
>> do soft reset again to port 0.
>> So the 'ahci_pmp_retry_srst_ops' is required when imx53
>> ahci is present.
>>
>> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
>> ---
> [...]
>> ? ? ? 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,
>> - ? ? };
>> + ? ? struct platform_device_id *id_entry = platform_get_device_id(pdev);
>> + ? ? struct ata_port_info pi = ahci_port_info[id_entry->driver_data];
>
> Why not pass port info via platform_data? It seems to be platform
> specific nowadays, so leave the default as is, but let the platforms
> pass their own port info through platform_data.

That's also a very clean way. However I have the concern that it might
end up with many duplicate entries.

>
> Thanks,
>
> --
> Anton Vorontsov
> Email: cbouatmailru at gmail.com
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Richard Zhu]

Hi:
Pass port info from the platform data is a good idea.
About the codes duplication in the board level, I think that we can
re-use the default  port info on most boards, when the ata_port_info
is null in the platform data struct.
We can leave the ahci_platform.c without any modifications in this way.

How do you guys think about?

Best Regard
Richard Zhu


On 29 August 2011 20:25, Eric Miao <eric.miao@linaro.org> wrote:
> On Mon, Aug 29, 2011 at 8:12 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
>> Hello,
>>
>> On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote:
>>> On imx53 AHCI, soft reset fails with IPMS set when PMP
>>> is enabled but SATA HDD/ODD is connected to SATA port,
>>> do soft reset again to port 0.
>>> So the 'ahci_pmp_retry_srst_ops' is required when imx53
>>> ahci is present.
>>>
>>> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
>>> ---
>> [...]
>>> ? ? ? 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,
>>> - ? ? };
>>> + ? ? struct platform_device_id *id_entry = platform_get_device_id(pdev);
>>> + ? ? struct ata_port_info pi = ahci_port_info[id_entry->driver_data];
>>
>> Why not pass port info via platform_data? It seems to be platform
>> specific nowadays, so leave the default as is, but let the platforms
>> pass their own port info through platform_data.
>
> That's also a very clean way. However I have the concern that it might
> end up with many duplicate entries.
>
>>
>> Thanks,
>>
>> --
>> Anton Vorontsov
>> Email: cbouatmailru at gmail.com
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Eric Miao]

On Tue, Aug 30, 2011 at 2:21 PM, Richard Zhu <richard.zhu@linaro.org> wrote:
> Hi:
> Pass port info from the platform data is a good idea.
> About the codes duplication in the board level, I think that we can
> re-use the default ?port info on most boards, when the ata_port_info
> is null in the platform data struct.
> We can leave the ahci_platform.c without any modifications in this way.
>
> How do you guys think about?

+1. Richard, please let's first go that way, so to have minimum impact to
ahci_platform.c

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Richard Zhu]

Hi:
One more concern, the .../drivers/ata/ahci.h file should be moved to
.../include/linux/ folder.
Because that the kinds of macros used in the ata_port_info and the
ahci_pmp_retry_srst_ops are
all defined in the .../drivers/ata/ahci.h file.
Otherwise, the ata_port_info can't be defined in the board related
platform data.

How we can handle this case?

Best Regard
Richard Zhu

On 30 August 2011 14:21, Richard Zhu <richard.zhu@linaro.org> wrote:
> Hi:
> Pass port info from the platform data is a good idea.
> About the codes duplication in the board level, I think that we can
> re-use the default ?port info on most boards, when the ata_port_info
> is null in the platform data struct.
> We can leave the ahci_platform.c without any modifications in this way.
>
> How do you guys think about?
>
> Best Regard
> Richard Zhu
>
>
> On 29 August 2011 20:25, Eric Miao <eric.miao@linaro.org> wrote:
>> On Mon, Aug 29, 2011 at 8:12 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
>>> Hello,
>>>
>>> On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote:
>>>> On imx53 AHCI, soft reset fails with IPMS set when PMP
>>>> is enabled but SATA HDD/ODD is connected to SATA port,
>>>> do soft reset again to port 0.
>>>> So the 'ahci_pmp_retry_srst_ops' is required when imx53
>>>> ahci is present.
>>>>
>>>> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
>>>> ---
>>> [...]
>>>> ? ? ? 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,
>>>> - ? ? };
>>>> + ? ? struct platform_device_id *id_entry = platform_get_device_id(pdev);
>>>> + ? ? struct ata_port_info pi = ahci_port_info[id_entry->driver_data];
>>>
>>> Why not pass port info via platform_data? It seems to be platform
>>> specific nowadays, so leave the default as is, but let the platforms
>>> pass their own port info through platform_data.
>>
>> That's also a very clean way. However I have the concern that it might
>> end up with many duplicate entries.
>>
>>>
>>> Thanks,
>>>
>>> --
>>> Anton Vorontsov
>>> Email: cbouatmailru at gmail.com
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>>
>

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Richard Zhu]

Hi:
Maybe we have to come back to take wolfram's suggestion.
Remove the mx53 specification in the ahci_type, and replace it by
 the generic one AHCI_PMP_RTY_SRST
Then the other ahci controller that needs the same PMP retry soft
reset SW work-around can reuse the
ata_port_info.

Hi Eric:
How about to use the generic ahci_type AHCI_PMP_RTY_SRST to replace
the IMX53_AHCI firstly?

The specified ahci_type can be added if the ahci controller needs some
different configurations refer to
the definitions in the AHCI_PMP_RTY_SRST's ata_port_info.

In this way, we can re-use the codes as much as possible.

Best Regards
Richard Zhu

On 30 August 2011 15:07, Richard Zhu <richard.zhu@linaro.org> wrote:
> Hi:
> One more concern, the .../drivers/ata/ahci.h file should be moved to
> .../include/linux/ folder.
> Because that the kinds of macros used in the ata_port_info and the
> ahci_pmp_retry_srst_ops are
> all defined in the .../drivers/ata/ahci.h file.
> Otherwise, the ata_port_info can't be defined in the board related
> platform data.
>
> How we can handle this case?
>
> Best Regard
> Richard Zhu
>
> On 30 August 2011 14:21, Richard Zhu <richard.zhu@linaro.org> wrote:
>> Hi:
>> Pass port info from the platform data is a good idea.
>> About the codes duplication in the board level, I think that we can
>> re-use the default ?port info on most boards, when the ata_port_info
>> is null in the platform data struct.
>> We can leave the ahci_platform.c without any modifications in this way.
>>
>> How do you guys think about?
>>
>> Best Regard
>> Richard Zhu
>>
>>
>> On 29 August 2011 20:25, Eric Miao <eric.miao@linaro.org> wrote:
>>> On Mon, Aug 29, 2011 at 8:12 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
>>>> Hello,
>>>>
>>>> On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote:
>>>>> On imx53 AHCI, soft reset fails with IPMS set when PMP
>>>>> is enabled but SATA HDD/ODD is connected to SATA port,
>>>>> do soft reset again to port 0.
>>>>> So the 'ahci_pmp_retry_srst_ops' is required when imx53
>>>>> ahci is present.
>>>>>
>>>>> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
>>>>> ---
>>>> [...]
>>>>> ? ? ? 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,
>>>>> - ? ? };
>>>>> + ? ? struct platform_device_id *id_entry = platform_get_device_id(pdev);
>>>>> + ? ? struct ata_port_info pi = ahci_port_info[id_entry->driver_data];
>>>>
>>>> Why not pass port info via platform_data? It seems to be platform
>>>> specific nowadays, so leave the default as is, but let the platforms
>>>> pass their own port info through platform_data.
>>>
>>> That's also a very clean way. However I have the concern that it might
>>> end up with many duplicate entries.
>>>
>>>>
>>>> Thanks,
>>>>
>>>> --
>>>> Anton Vorontsov
>>>> Email: cbouatmailru at gmail.com
>>>>
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel at lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>>
>>>
>>
>

[PATCH V6 2/5] ahci_plt Add the board_ids and pi refer to different features

[Eric Miao]

On Tue, Aug 30, 2011 at 3:07 PM, Richard Zhu <richard.zhu@linaro.org> wrote:
> Hi:
> One more concern, the .../drivers/ata/ahci.h file should be moved to
> .../include/linux/ folder.
> Because that the kinds of macros used in the ata_port_info and the
> ahci_pmp_retry_srst_ops are
> all defined in the .../drivers/ata/ahci.h file.
> Otherwise, the ata_port_info can't be defined in the board related
> platform data.
>
> How we can handle this case?

That's indeed an issue, Anton, what's your suggestion?