From acbd573354bb7b7b7a3891018a39f4b3976b0c43 Mon Sep 17 00:00:00 2001 From: Mikko Perttunen Date: Tue, 17 Jun 2014 15:07:55 +0300 Subject: [PATCH 1/6] libahci_platform: Fail when PHY required but PHY support disabled ahci_platform_get_resources handles resource management for platform AHCI drivers, including getting a possible PHY from the device tree. Since not all drivers need a PHY, it ignores -ENODEV and -ENOSYS from devm_get_phy. However, when the PHY subsystem is mistakenly disabled, -ENOSYS can be returned even when a PHY is needed. This patch modifies the -ENOSYS case to check if a "phys" device tree node exists. If it exists, then clearly the PHY subsystem is mistakenly disabled and the driver cannot work, ahci_platform_get_resources will fail and propagate the error. Signed-off-by: Mikko Perttunen Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: Tejun Heo --- drivers/ata/libahci_platform.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c index 3a5b4ed25a4f..b0077589f065 100644 --- a/drivers/ata/libahci_platform.c +++ b/drivers/ata/libahci_platform.c @@ -250,8 +250,13 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev) if (IS_ERR(hpriv->phy)) { rc = PTR_ERR(hpriv->phy); switch (rc) { - case -ENODEV: case -ENOSYS: + /* No PHY support. Check if PHY is required. */ + if (of_find_property(dev->of_node, "phys", NULL)) { + dev_err(dev, "couldn't get sata-phy: ENOSYS\n"); + goto err_out; + } + case -ENODEV: /* continue normally */ hpriv->phy = NULL; break; From e6dd42a917e62d916c6e513dbf87a4dec8cf3a1c Mon Sep 17 00:00:00 2001 From: Shawn Guo Date: Wed, 28 May 2014 23:05:39 +0800 Subject: [PATCH 2/6] ahci: imx: manage only sata_ref_clk in imx_sata_enable[disable] Doing suspend/resume on imx6q and imx53 boards with no SATA disk attached will trigger the following warning. ------------[ cut here ]------------ WARNING: CPU: 0 PID: 661 at drivers/ata/libahci.c:224 ahci_enable_ahci+0x74/0x8) Modules linked in: CPU: 0 PID: 661 Comm: sh Tainted: G W 3.15.0-rc5-next-20140521-000027 Backtrace: [<80011c90>] (dump_backtrace) from [<80011e2c>] (show_stack+0x18/0x1c) r6:803a22f4 r5:00000000 r4:00000000 r3:00000000 [<80011e14>] (show_stack) from [<80661e60>] (dump_stack+0x88/0xa4) [<80661dd8>] (dump_stack) from [<80028fdc>] (warn_slowpath_common+0x70/0x94) r5:00000009 r4:00000000 [<80028f6c>] (warn_slowpath_common) from [<80029024>] (warn_slowpath_null+0x24/) r8:808f68c4 r7:00000000 r6:00000000 r5:00000000 r4:e0810004 [<80029000>] (warn_slowpath_null) from [<803a22f4>] (ahci_enable_ahci+0x74/0x80) [<803a2280>] (ahci_enable_ahci) from [<803a2324>] (ahci_reset_controller+0x24/0) r8:ddcd9410 r7:80351178 r6:ddcd9444 r5:dde8b850 r4:e0810000 r3:ddf35e90 [<803a2300>] (ahci_reset_controller) from [<803a2c68>] (ahci_platform_resume_ho) r7:80351178 r6:ddcd9444 r5:dde8b850 r4:ddcd9410 [<803a2c30>] (ahci_platform_resume_host) from [<803a38f0>] (imx_ahci_resume+0x2) r5:00000000 r4:ddcd9410 [<803a38c4>] (imx_ahci_resume) from [<803511ac>] (platform_pm_resume+0x34/0x54) .... The reason is that the SATA controller has no working clock at this point, and thus ahci_enable_ahci() fails to enable the controller. In case that there is no SATA disk attached, the imx_sata_disable() gets called in ahci_imx_error_handler(), and both sata_clk and sata_ref_clk will be disabled there. Because all the imx_sata_enable() calls afterward will return immediately due to imxpriv->no_device check, the SATA controller working clock sata_clk will never get any chance to be enabled again. This is a regression caused by commit 90870d79d4f2 (ahci-imx: Port to library-ised ahci_platform). Before the commit, only sata_ref_clk is managed by the driver in enable/disable function. But after the commit, all the clocks are enabled/disabled in a row by ahci platform helpers ahci_platform_enable[disable]_clks. Since ahb_clk is a bus clock which does not have gate at all, and i.MX low-power hardware module already manages sata_clk across suspend/resume cycle, the only clock that needs to be managed by software is sata_ref_clk. So instead of using ahci_platform_enable[disable]_clks to manage all the clocks in a row from imx_sata_enable[disable], we should manage only sata_ref_clk in there. Reported-by: Fabio Estevam Fixes: 90870d79d4f2 (ahci-imx: Port to library-ised ahci_platform) Signed-off-by: Shawn Guo Acked-by: Hans de Goede --- drivers/ata/ahci_imx.c | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index 3a901520c62b..4384a7d72133 100644 --- a/drivers/ata/ahci_imx.c +++ b/drivers/ata/ahci_imx.c @@ -58,6 +58,8 @@ enum ahci_imx_type { struct imx_ahci_priv { struct platform_device *ahci_pdev; enum ahci_imx_type type; + struct clk *sata_clk; + struct clk *sata_ref_clk; struct clk *ahb_clk; struct regmap *gpr; bool no_device; @@ -224,7 +226,7 @@ static int imx_sata_enable(struct ahci_host_priv *hpriv) return ret; } - ret = ahci_platform_enable_clks(hpriv); + ret = clk_prepare_enable(imxpriv->sata_ref_clk); if (ret < 0) goto disable_regulator; @@ -291,7 +293,7 @@ static void imx_sata_disable(struct ahci_host_priv *hpriv) !IMX6Q_GPR13_SATA_MPLL_CLK_EN); } - ahci_platform_disable_clks(hpriv); + clk_disable_unprepare(imxpriv->sata_ref_clk); if (hpriv->target_pwr) regulator_disable(hpriv->target_pwr); @@ -385,6 +387,19 @@ static int imx_ahci_probe(struct platform_device *pdev) imxpriv->no_device = false; imxpriv->first_time = true; imxpriv->type = (enum ahci_imx_type)of_id->data; + + imxpriv->sata_clk = devm_clk_get(dev, "sata"); + if (IS_ERR(imxpriv->sata_clk)) { + dev_err(dev, "can't get sata clock.\n"); + return PTR_ERR(imxpriv->sata_clk); + } + + imxpriv->sata_ref_clk = devm_clk_get(dev, "sata_ref"); + if (IS_ERR(imxpriv->sata_ref_clk)) { + dev_err(dev, "can't get sata_ref clock.\n"); + return PTR_ERR(imxpriv->sata_ref_clk); + } + imxpriv->ahb_clk = devm_clk_get(dev, "ahb"); if (IS_ERR(imxpriv->ahb_clk)) { dev_err(dev, "can't get ahb clock.\n"); @@ -407,10 +422,14 @@ static int imx_ahci_probe(struct platform_device *pdev) hpriv->plat_data = imxpriv; - ret = imx_sata_enable(hpriv); + ret = clk_prepare_enable(imxpriv->sata_clk); if (ret) return ret; + ret = imx_sata_enable(hpriv); + if (ret) + goto disable_clk; + /* * Configure the HWINIT bits of the HOST_CAP and HOST_PORTS_IMPL, * and IP vendor specific register IMX_TIMER1MS. @@ -435,16 +454,24 @@ static int imx_ahci_probe(struct platform_device *pdev) ret = ahci_platform_init_host(pdev, hpriv, &ahci_imx_port_info, 0, 0, 0); if (ret) - imx_sata_disable(hpriv); + goto disable_sata; + return 0; + +disable_sata: + imx_sata_disable(hpriv); +disable_clk: + clk_disable_unprepare(imxpriv->sata_clk); return ret; } static void ahci_imx_host_stop(struct ata_host *host) { struct ahci_host_priv *hpriv = host->private_data; + struct imx_ahci_priv *imxpriv = hpriv->plat_data; imx_sata_disable(hpriv); + clk_disable_unprepare(imxpriv->sata_clk); } #ifdef CONFIG_PM_SLEEP From 72cbaa3d2f563d7b48c9f8aef47ec9aa3a31adf2 Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Tue, 24 Jun 2014 16:54:23 -0400 Subject: [PATCH 3/6] ahci: disable ncq feature for hisilicon sata NCQ feature is unsupported on hisilicon sata controller, so disable it. This version of IP is used by hip04 and hix5hd2 soc. tj: "|=" was replaced with "=" for no reason. Restored "|=". Signed-off-by: Kefeng Wang Sigend-off-by: Tejun Heo --- drivers/ata/ahci_platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c index ebe505c17763..b10d81ddb528 100644 --- a/drivers/ata/ahci_platform.c +++ b/drivers/ata/ahci_platform.c @@ -58,7 +58,7 @@ static int ahci_probe(struct platform_device *pdev) } if (of_device_is_compatible(dev->of_node, "hisilicon,hisi-ahci")) - hflags |= AHCI_HFLAG_NO_FBS; + hflags |= AHCI_HFLAG_NO_FBS | AHCI_HFLAG_NO_NCQ; rc = ahci_platform_init_host(pdev, hpriv, &ahci_port_info, hflags, 0, 0); From f118ae5901172dacc4f272acf5eccfba06e8d221 Mon Sep 17 00:00:00 2001 From: Russell King Date: Mon, 23 Jun 2014 12:59:08 +0100 Subject: [PATCH 4/6] ata: ahci_imx: warn when disabling ahci link When the AHCI link is disabled, it can't be re-enabled except by resetting the entire SoC. Rather than doing this silently print some kernel messages to inform the user, along with how to avoid this. tj: Put a long printf format string on a single line. Signed-off-by: Russell King Signed-off-by: Tejun Heo --- drivers/ata/ahci_imx.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index 4384a7d72133..cac4360f272a 100644 --- a/drivers/ata/ahci_imx.c +++ b/drivers/ata/ahci_imx.c @@ -326,6 +326,9 @@ static void ahci_imx_error_handler(struct ata_port *ap) writel(reg_val | IMX_P0PHYCR_TEST_PDDQ, mmio + IMX_P0PHYCR); imx_sata_disable(hpriv); imxpriv->no_device = true; + + dev_info(ap->dev, "no device found, disabling link.\n"); + dev_info(ap->dev, "pass " MODULE_PARAM_PREFIX ".hotplug=1 to enable hotplug\n"); } static int ahci_imx_softreset(struct ata_link *link, unsigned int *class, From 39e0ee9964b1245b79ec89f6b89d8ec4ef672524 Mon Sep 17 00:00:00 2001 From: Suman Tripathi Date: Mon, 7 Jul 2014 22:33:04 +0530 Subject: [PATCH 5/6] libahci: export ahci_qc_issue() and ahci_start_fix_rx() The subsequent patch will make use of them. Signed-off-by: Loc Ho Signed-off-by: Suman Tripathi Signed-off-by: Tejun Heo --- drivers/ata/ahci.h | 2 ++ drivers/ata/libahci.c | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h index 05882e4445a6..5513296e5e2e 100644 --- a/drivers/ata/ahci.h +++ b/drivers/ata/ahci.h @@ -371,7 +371,9 @@ int ahci_do_softreset(struct ata_link *link, unsigned int *class, int pmp, unsigned long deadline, int (*check_ready)(struct ata_link *link)); +unsigned int ahci_qc_issue(struct ata_queued_cmd *qc); int ahci_stop_engine(struct ata_port *ap); +void ahci_start_fis_rx(struct ata_port *ap); void ahci_start_engine(struct ata_port *ap); int ahci_check_ready(struct ata_link *link); int ahci_kick_engine(struct ata_port *ap); diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index 40ea583d3610..d72ce0470309 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -68,7 +68,6 @@ static ssize_t ahci_transmit_led_message(struct ata_port *ap, u32 state, static int ahci_scr_read(struct ata_link *link, unsigned int sc_reg, u32 *val); static int ahci_scr_write(struct ata_link *link, unsigned int sc_reg, u32 val); -static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc); static bool ahci_qc_fill_rtf(struct ata_queued_cmd *qc); static int ahci_port_start(struct ata_port *ap); static void ahci_port_stop(struct ata_port *ap); @@ -620,7 +619,7 @@ int ahci_stop_engine(struct ata_port *ap) } EXPORT_SYMBOL_GPL(ahci_stop_engine); -static void ahci_start_fis_rx(struct ata_port *ap) +void ahci_start_fis_rx(struct ata_port *ap) { void __iomem *port_mmio = ahci_port_base(ap); struct ahci_host_priv *hpriv = ap->host->private_data; @@ -646,6 +645,7 @@ static void ahci_start_fis_rx(struct ata_port *ap) /* flush */ readl(port_mmio + PORT_CMD); } +EXPORT_SYMBOL_GPL(ahci_start_fis_rx); static int ahci_stop_fis_rx(struct ata_port *ap) { @@ -1945,7 +1945,7 @@ irqreturn_t ahci_interrupt(int irq, void *dev_instance) } EXPORT_SYMBOL_GPL(ahci_interrupt); -static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc) +unsigned int ahci_qc_issue(struct ata_queued_cmd *qc) { struct ata_port *ap = qc->ap; void __iomem *port_mmio = ahci_port_base(ap); @@ -1974,6 +1974,7 @@ static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc) return 0; } +EXPORT_SYMBOL_GPL(ahci_qc_issue); static bool ahci_qc_fill_rtf(struct ata_queued_cmd *qc) { From 2a0bdff6b958d1b2523d2754b6cd5e0ea4053016 Mon Sep 17 00:00:00 2001 From: Suman Tripathi Date: Mon, 7 Jul 2014 22:33:05 +0530 Subject: [PATCH 6/6] ahci_xgene: fix the dma state machine lockup for the IDENTIFY DEVICE PIO mode command. This patch fixes the dma state machine lockup due to the processing of IDENTIFY DEVICE PIO mode command. The X-Gene AHCI controller has an errata in which it cannot clear the BSY bit after the PIO setup FIS. The dma state machine enters CMFatalErrorUpdate state and locks up. This patch also removes the dma restart workaround from the read_id function as the read_id function is only called by libata layer for ATA_INTERNAL commands. But for some cases eg: PORT MULTIPLIER and udev, the framework will enumerate using SCSI commands and it will not call read_id function. Signed-off-by: Loc Ho Signed-off-by: Suman Tripathi Signed-off-by: Tejun Heo --- drivers/ata/ahci_xgene.c | 60 +++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c index 042a9bb45c86..ee3a3659bd9e 100644 --- a/drivers/ata/ahci_xgene.c +++ b/drivers/ata/ahci_xgene.c @@ -78,6 +78,7 @@ struct xgene_ahci_context { struct ahci_host_priv *hpriv; struct device *dev; + u8 last_cmd[MAX_AHCI_CHN_PERCTR]; /* tracking the last command issued*/ void __iomem *csr_core; /* Core CSR address of IP */ void __iomem *csr_diag; /* Diag CSR address of IP */ void __iomem *csr_axi; /* AXI CSR address of IP */ @@ -97,6 +98,50 @@ static int xgene_ahci_init_memram(struct xgene_ahci_context *ctx) return 0; } +/** + * xgene_ahci_restart_engine - Restart the dma engine. + * @ap : ATA port of interest + * + * Restarts the dma engine inside the controller. + */ +static int xgene_ahci_restart_engine(struct ata_port *ap) +{ + struct ahci_host_priv *hpriv = ap->host->private_data; + + ahci_stop_engine(ap); + ahci_start_fis_rx(ap); + hpriv->start_engine(ap); + + return 0; +} + +/** + * xgene_ahci_qc_issue - Issue commands to the device + * @qc: Command to issue + * + * Due to Hardware errata for IDENTIFY DEVICE command, the controller cannot + * clear the BSY bit after receiving the PIO setup FIS. This results in the dma + * state machine goes into the CMFatalErrorUpdate state and locks up. By + * restarting the dma engine, it removes the controller out of lock up state. + */ +static unsigned int xgene_ahci_qc_issue(struct ata_queued_cmd *qc) +{ + struct ata_port *ap = qc->ap; + struct ahci_host_priv *hpriv = ap->host->private_data; + struct xgene_ahci_context *ctx = hpriv->plat_data; + int rc = 0; + + if (unlikely(ctx->last_cmd[ap->port_no] == ATA_CMD_ID_ATA)) + xgene_ahci_restart_engine(ap); + + rc = ahci_qc_issue(qc); + + /* Save the last command issued */ + ctx->last_cmd[ap->port_no] = qc->tf.command; + + return rc; +} + /** * xgene_ahci_read_id - Read ID data from the specified device * @dev: device @@ -104,14 +149,12 @@ static int xgene_ahci_init_memram(struct xgene_ahci_context *ctx) * @id: data buffer * * This custom read ID function is required due to the fact that the HW - * does not support DEVSLP and the controller state machine may get stuck - * after processing the ID query command. + * does not support DEVSLP. */ static unsigned int xgene_ahci_read_id(struct ata_device *dev, struct ata_taskfile *tf, u16 *id) { u32 err_mask; - void __iomem *port_mmio = ahci_port_base(dev->link->ap); err_mask = ata_do_dev_read_id(dev, tf, id); if (err_mask) @@ -133,16 +176,6 @@ static unsigned int xgene_ahci_read_id(struct ata_device *dev, */ id[ATA_ID_FEATURE_SUPP] &= ~(1 << 8); - /* - * Due to HW errata, restart the port if no other command active. - * Otherwise the controller may get stuck. - */ - if (!readl(port_mmio + PORT_CMD_ISSUE)) { - writel(PORT_CMD_FIS_RX, port_mmio + PORT_CMD); - readl(port_mmio + PORT_CMD); /* Force a barrier */ - writel(PORT_CMD_FIS_RX | PORT_CMD_START, port_mmio + PORT_CMD); - readl(port_mmio + PORT_CMD); /* Force a barrier */ - } return 0; } @@ -300,6 +333,7 @@ static struct ata_port_operations xgene_ahci_ops = { .host_stop = xgene_ahci_host_stop, .hardreset = xgene_ahci_hardreset, .read_id = xgene_ahci_read_id, + .qc_issue = xgene_ahci_qc_issue, }; static const struct ata_port_info xgene_ahci_port_info = {