Linux-ide Archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] ahci: imx: software workaround for phy reset issue
@ 2014-04-18  6:44 Shawn Guo
  2014-04-18  6:44 ` [PATCH v2 1/2] ahci: imx: add namespace for register enums Shawn Guo
  2014-04-18  6:44 ` [PATCH v2 2/2] ahci: imx: software workaround for phy reset issue in resume Shawn Guo
  0 siblings, 2 replies; 4+ messages in thread
From: Shawn Guo @ 2014-04-18  6:44 UTC (permalink / raw
  To: linux-ide; +Cc: Tejun Heo, linux-arm-kernel, Shawn Guo

Changes since v1:
 - Do not switch to defines and stay with enums
 - Create a wrapper function imx_phy_crbit_assert() to make the code
   a little shorter and easier for eyes

Shawn Guo (2):
  ahci: imx: add namespace for register enums
  ahci: imx: software workaround for phy reset issue in resume

 drivers/ata/ahci_imx.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 165 insertions(+), 7 deletions(-)

-- 
1.8.3.2



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/2] ahci: imx: add namespace for register enums
  2014-04-18  6:44 [PATCH v2 0/2] ahci: imx: software workaround for phy reset issue Shawn Guo
@ 2014-04-18  6:44 ` Shawn Guo
  2014-05-02 15:42   ` Tejun Heo
  2014-04-18  6:44 ` [PATCH v2 2/2] ahci: imx: software workaround for phy reset issue in resume Shawn Guo
  1 sibling, 1 reply; 4+ messages in thread
From: Shawn Guo @ 2014-04-18  6:44 UTC (permalink / raw
  To: linux-ide; +Cc: Tejun Heo, linux-arm-kernel, Shawn Guo

Update register enums a little bit to add proper namespace prefix, and
have the names match i.MX reference manual.

Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
 drivers/ata/ahci_imx.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 497c7ab..009a074 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -29,9 +29,9 @@
 #include "ahci.h"
 
 enum {
-	PORT_PHY_CTL = 0x178,			/* Port0 PHY Control */
-	PORT_PHY_CTL_PDDQ_LOC = 0x100000,	/* PORT_PHY_CTL bits */
-	HOST_TIMER1MS = 0xe0,			/* Timer 1-ms */
+	IMX_SATA_TIMER1MS			= 0x00e0,
+	IMX_SATA_P0PHYCR			= 0x0178,
+	IMX_SATA_P0PHYCR_TEST_PDDQ		= 1 << 20,
 };
 
 enum ahci_imx_type {
@@ -156,8 +156,8 @@ static void ahci_imx_error_handler(struct ata_port *ap)
 	 * without full reset once the pddq mode is enabled making it
 	 * impossible to use as part of libata LPM.
 	 */
-	reg_val = readl(mmio + PORT_PHY_CTL);
-	writel(reg_val | PORT_PHY_CTL_PDDQ_LOC, mmio + PORT_PHY_CTL);
+	reg_val = readl(mmio + IMX_SATA_P0PHYCR);
+	writel(reg_val | IMX_SATA_P0PHYCR_TEST_PDDQ, mmio + IMX_SATA_P0PHYCR);
 	imx_sata_disable(hpriv);
 	imxpriv->no_device = true;
 }
@@ -248,7 +248,7 @@ static int imx_ahci_probe(struct platform_device *pdev)
 
 	/*
 	 * Configure the HWINIT bits of the HOST_CAP and HOST_PORTS_IMPL,
-	 * and IP vendor specific register HOST_TIMER1MS.
+	 * and IP vendor specific register IMX_SATA_TIMER1MS.
 	 * Configure CAP_SSS (support stagered spin up).
 	 * Implement the port0.
 	 * Get the ahb clock rate, and configure the TIMER1MS register.
@@ -265,7 +265,7 @@ static int imx_ahci_probe(struct platform_device *pdev)
 	}
 
 	reg_val = clk_get_rate(imxpriv->ahb_clk) / 1000;
-	writel(reg_val, hpriv->mmio + HOST_TIMER1MS);
+	writel(reg_val, hpriv->mmio + IMX_SATA_TIMER1MS);
 
 	ret = ahci_platform_init_host(pdev, hpriv, &ahci_imx_port_info, 0, 0);
 	if (ret)
-- 
1.8.3.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] ahci: imx: software workaround for phy reset issue in resume
  2014-04-18  6:44 [PATCH v2 0/2] ahci: imx: software workaround for phy reset issue Shawn Guo
  2014-04-18  6:44 ` [PATCH v2 1/2] ahci: imx: add namespace for register enums Shawn Guo
@ 2014-04-18  6:44 ` Shawn Guo
  1 sibling, 0 replies; 4+ messages in thread
From: Shawn Guo @ 2014-04-18  6:44 UTC (permalink / raw
  To: linux-ide; +Cc: Tejun Heo, linux-arm-kernel, Shawn Guo, Richard Zhu

When suspending imx6q systems which have rootfs on SATA, the following
error will likely be seen in resume.  The SATA link will fail to come
up, and it results in an unusable system across the suspend/resume
cycle.

$ echo mem > /sys/power/state
PM: Syncing filesystems ... done.
PM: Preparing system for mem sleep
Freezing user space processes ... (elapsed 0.002 seconds) done.
Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done.
PM: Entering mem sleep
sd 0:0:0:0: [sda] Synchronizing SCSI cache
sd 0:0:0:0: [sda] Stopping disk
PM: suspend of devices complete after 61.914 msecs
PM: suspend devices took 0.070 seconds
PM: late suspend of devices complete after 4.906 msecs
PM: noirq suspend of devices complete after 4.521 msecs
Disabling non-boot CPUs ...
CPU1: shutdown
CPU2: shutdown
CPU3: shutdown
Enabling non-boot CPUs ...
CPU1: Booted secondary processor
CPU1 is up
CPU2: Booted secondary processor
CPU2 is up
CPU3: Booted secondary processor
CPU3 is up
PM: noirq resume of devices complete after 10.486 msecs
PM: early resume of devices complete after 4.679 msecs
sd 0:0:0:0: [sda] Starting disk
PM: resume of devices complete after 22.674 msecs
PM: resume devices took 0.030 seconds
PM: Finishing wakeup.
Restarting tasks ... done.
$ ata1: SATA link down (SStatus 1 SControl 300)
ata1: SATA link down (SStatus 1 SControl 300)
ata1: limiting SATA link speed to 1.5 Gbps
ata1: SATA link down (SStatus 1 SControl 310)
ata1.00: disabled
ata1: exception Emask 0x10 SAct 0x0 SErr 0x4040000 action 0xe frozen t4
ata1: irq_stat 0x00000040, connection status changed
ata1: SError: { CommWake DevExch }
ata1: hard resetting link
sd 0:0:0:0: rejecting I/O to offline device
sd 0:0:0:0: killing request
sd 0:0:0:0: rejecting I/O to offline device
Aborting journal on device sda2-8.
sd 0:0:0:0: rejecting I/O to offline device
EXT4-fs warning (device sda2): ext4_end_bio:317: I/O error writing to inode 132577 (offset 0 size 0 starting block 26235)
Buffer I/O error on device sda2, logical block 10169
...

It's caused by a silicon issue that SATA phy does not get reset by
controller when coming back from LPM.  The patch adds a software
workaround for this issue.  It enforces a software reset on SATA phy
in imx_sata_enable() function, so that we can ensure SATA link will
come up properly in both power-on and resume.

The software reset is implemented by writing phy reset register through
the phy control register bus interface.  Functions
imx_phy_reg_[addressing|write|read]() implement this bus interface, while
imx_sata_phy_reset() performs the actually reset operation.

Signed-off-by: Richard Zhu <r65037@freescale.com>
Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
 drivers/ata/ahci_imx.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 009a074..1c3d4c8 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -32,6 +32,17 @@ enum {
 	IMX_SATA_TIMER1MS			= 0x00e0,
 	IMX_SATA_P0PHYCR			= 0x0178,
 	IMX_SATA_P0PHYCR_TEST_PDDQ		= 1 << 20,
+	IMX_SATA_P0PHYCR_CR_READ		= 1 << 19,
+	IMX_SATA_P0PHYCR_CR_WRITE		= 1 << 18,
+	IMX_SATA_P0PHYCR_CR_CAP_DATA		= 1 << 17,
+	IMX_SATA_P0PHYCR_CR_CAP_ADDR		= 1 << 16,
+	IMX_SATA_P0PHYSR			= 0x017c,
+	IMX_SATA_P0PHYSR_CR_ACK			= 1 << 18,
+	IMX_SATA_P0PHYSR_CR_DATA_OUT		= 0xffff << 0,
+	IMX_PHY_LANE0_OUT_STAT			= 0x2003,
+	IMX_PHY_LANE0_OUT_STAT_RX_PLL_STATE	= 1 << 1,
+	IMX_PHY_CLOCK_RESET			= 0x7f3f,
+	IMX_PHY_CLOCK_RESET_RESET		= 1 << 0,
 };
 
 enum ahci_imx_type {
@@ -54,9 +65,149 @@ MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support, 1=support
 
 static void ahci_imx_host_stop(struct ata_host *host);
 
+static int imx_phy_crbit_assert(void __iomem *mmio, u32 bit, bool assert)
+{
+	int timeout = 10;
+	u32 crval;
+	u32 srval;
+
+	/* Assert or deassert the bit */
+	crval = readl(mmio + IMX_SATA_P0PHYCR);
+	if (assert)
+		crval |= bit;
+	else
+		crval &= ~bit;
+	writel(crval, mmio + IMX_SATA_P0PHYCR);
+
+	/* Wait for the cr_ack signal */
+	do {
+		srval = readl(mmio + IMX_SATA_P0PHYSR);
+		if ((assert ? srval : ~srval) & IMX_SATA_P0PHYSR_CR_ACK)
+			break;
+		usleep_range(100, 200);
+	} while (--timeout);
+
+	return timeout ? 0 : -ETIMEDOUT;
+}
+
+static int imx_phy_reg_addressing(u16 addr, void __iomem *mmio)
+{
+	u32 crval = addr;
+	int ret;
+
+	/* Supply the address on cr_data_in */
+	writel(crval, mmio + IMX_SATA_P0PHYCR);
+
+	/* Assert the cr_cap_addr signal */
+	ret = imx_phy_crbit_assert(mmio, IMX_SATA_P0PHYCR_CR_CAP_ADDR, true);
+	if (ret)
+		return ret;
+
+	/* Deassert cr_cap_addr */
+	ret = imx_phy_crbit_assert(mmio, IMX_SATA_P0PHYCR_CR_CAP_ADDR, false);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int imx_phy_reg_write(u16 val, void __iomem *mmio)
+{
+	u32 crval = val;
+	int ret;
+
+	/* Supply the data on cr_data_in */
+	writel(crval, mmio + IMX_SATA_P0PHYCR);
+
+	/* Assert the cr_cap_data signal */
+	ret = imx_phy_crbit_assert(mmio, IMX_SATA_P0PHYCR_CR_CAP_DATA, true);
+	if (ret)
+		return ret;
+
+	/* Deassert cr_cap_data */
+	ret = imx_phy_crbit_assert(mmio, IMX_SATA_P0PHYCR_CR_CAP_DATA, false);
+	if (ret)
+		return ret;
+
+	if (val & IMX_PHY_CLOCK_RESET_RESET) {
+		/*
+		 * In case we're resetting the phy, it's unable to acknowledge,
+		 * so we return immediately here.
+		 */
+		crval |= IMX_SATA_P0PHYCR_CR_WRITE;
+		writel(crval, mmio + IMX_SATA_P0PHYCR);
+		goto out;
+	}
+
+	/* Assert the cr_write signal */
+	ret = imx_phy_crbit_assert(mmio, IMX_SATA_P0PHYCR_CR_WRITE, true);
+	if (ret)
+		return ret;
+
+	/* Deassert cr_write */
+	ret = imx_phy_crbit_assert(mmio, IMX_SATA_P0PHYCR_CR_WRITE, false);
+	if (ret)
+		return ret;
+
+out:
+	return 0;
+}
+
+static int imx_phy_reg_read(u16 *val, void __iomem *mmio)
+{
+	int ret;
+
+	/* Assert the cr_read signal */
+	ret = imx_phy_crbit_assert(mmio, IMX_SATA_P0PHYCR_CR_READ, true);
+	if (ret)
+		return ret;
+
+	/* Capture the data from cr_data_out[] */
+	*val = readl(mmio + IMX_SATA_P0PHYSR) & IMX_SATA_P0PHYSR_CR_DATA_OUT;
+
+	/* Deassert cr_read */
+	ret = imx_phy_crbit_assert(mmio, IMX_SATA_P0PHYCR_CR_READ, false);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int imx_sata_phy_reset(struct ahci_host_priv *hpriv)
+{
+	void __iomem *mmio = hpriv->mmio;
+	int timeout = 10;
+	u16 val;
+	int ret;
+
+	/* Reset SATA PHY by setting RESET bit of PHY register CLOCK_RESET */
+	ret = imx_phy_reg_addressing(IMX_PHY_CLOCK_RESET, mmio);
+	if (ret)
+		return ret;
+	ret = imx_phy_reg_write(IMX_PHY_CLOCK_RESET_RESET, mmio);
+	if (ret)
+		return ret;
+
+	/* Wait for PHY RX_PLL to be stable */
+	do {
+		usleep_range(100, 200);
+		ret = imx_phy_reg_addressing(IMX_PHY_LANE0_OUT_STAT, mmio);
+		if (ret)
+			return ret;
+		ret = imx_phy_reg_read(&val, mmio);
+		if (ret)
+			return ret;
+		if (val & IMX_PHY_LANE0_OUT_STAT_RX_PLL_STATE)
+			break;
+	} while (--timeout);
+
+	return timeout ? 0 : -ETIMEDOUT;
+}
+
 static int imx_sata_enable(struct ahci_host_priv *hpriv)
 {
 	struct imx_ahci_priv *imxpriv = hpriv->plat_data;
+	struct device *dev = &imxpriv->ahci_pdev->dev;
 	int ret;
 
 	if (imxpriv->no_device)
@@ -101,6 +252,12 @@ static int imx_sata_enable(struct ahci_host_priv *hpriv)
 		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
 				   IMX6Q_GPR13_SATA_MPLL_CLK_EN,
 				   IMX6Q_GPR13_SATA_MPLL_CLK_EN);
+
+		ret = imx_sata_phy_reset(hpriv);
+		if (ret) {
+			dev_err(dev, "failed to reset phy: %d\n", ret);
+			goto disable_regulator;
+		}
 	}
 
 	usleep_range(1000, 2000);
@@ -217,6 +374,7 @@ static int imx_ahci_probe(struct platform_device *pdev)
 	if (!imxpriv)
 		return -ENOMEM;
 
+	imxpriv->ahci_pdev = pdev;
 	imxpriv->no_device = false;
 	imxpriv->first_time = true;
 	imxpriv->type = (enum ahci_imx_type)of_id->data;
-- 
1.8.3.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/2] ahci: imx: add namespace for register enums
  2014-04-18  6:44 ` [PATCH v2 1/2] ahci: imx: add namespace for register enums Shawn Guo
@ 2014-05-02 15:42   ` Tejun Heo
  0 siblings, 0 replies; 4+ messages in thread
From: Tejun Heo @ 2014-05-02 15:42 UTC (permalink / raw
  To: Shawn Guo; +Cc: linux-ide, linux-arm-kernel

On Fri, Apr 18, 2014 at 02:44:25PM +0800, Shawn Guo wrote:
> Update register enums a little bit to add proper namespace prefix, and
> have the names match i.MX reference manual.
> 
> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> ---
>  drivers/ata/ahci_imx.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index 497c7ab..009a074 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -29,9 +29,9 @@
>  #include "ahci.h"
>  
>  enum {
> -	PORT_PHY_CTL = 0x178,			/* Port0 PHY Control */
> -	PORT_PHY_CTL_PDDQ_LOC = 0x100000,	/* PORT_PHY_CTL bits */
> -	HOST_TIMER1MS = 0xe0,			/* Timer 1-ms */
> +	IMX_SATA_TIMER1MS			= 0x00e0,
> +	IMX_SATA_P0PHYCR			= 0x0178,

P0PHYCR isn't really readily dechipherable to "Port0 PHY Control".
Probably keeping the comment is better?  Also, constant names don't
necessarily have to be exactly the same as hardware manuals.  If
easier to understand and map back (via comment or whatever), using
more humanly-readable names is completely fine.  Also, maybe IMX_SATA_
is a bit too long for prefix?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-05-02 15:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-18  6:44 [PATCH v2 0/2] ahci: imx: software workaround for phy reset issue Shawn Guo
2014-04-18  6:44 ` [PATCH v2 1/2] ahci: imx: add namespace for register enums Shawn Guo
2014-05-02 15:42   ` Tejun Heo
2014-04-18  6:44 ` [PATCH v2 2/2] ahci: imx: software workaround for phy reset issue in resume Shawn Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).