[PATCH v1 0/3] ata: Fixes related to APM X-Gene SATA host controller driver.

[PATCH v1 0/3] ata: Fixes related to APM X-Gene SATA host controller driver.

[Suman Tripathi]

This patch set contains a couple of fixes related to APM X-Gene SATA
controller driver.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---
Suman Tripathi (3):
  ata: Fix the watermark threshold for the APM X-Gene SATA host
    controller driver.
  ata: Fix the link down in first attempt for the APM X-Gene SoC AHCI
    SATA host controller driver.
  arm64: Fix the APM X-Gene SoC SATA PHY clock DTS node csr-mask of the
    SATA Host Controller 1.

 arch/arm64/boot/dts/apm-storm.dtsi |  4 ++--
 drivers/ata/ahci_xgene.c           | 25 ++++++++++++++++++++-----
 2 files changed, 22 insertions(+), 7 deletions(-)

--
1.8.2.1

[PATCH v1 1/3] ata: Fix the watermark threshold for the APM X-Gene SATA host controller driver.

[Suman Tripathi]

This patch fixes the watermark threshold of the receive FIFO for the
APM X-Gene SATA host controller driver.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---
 drivers/ata/ahci_xgene.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 77c89bf..03b6b0f 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -67,6 +67,9 @@
 #define PORTAXICFG			0x000000bc
 #define PORTAXICFG_OUTTRANS_SET(dst, src) \
 		(((dst) & ~0x00f00000) | (((u32)(src) << 0x14) & 0x00f00000))
+#define PORTRANSCFG			0x000000c8
+#define PORTRANSCFG_RXWM_SET(dst, src)		\
+		(((dst) & ~0x0000007f) | (((u32)(src)) & 0x0000007f))

 /* SATA host controller AXI CSR */
 #define INT_SLV_TMOMASK			0x00000010
@@ -176,6 +179,10 @@ static void xgene_ahci_set_phy_cfg(struct xgene_ahci_context *ctx, int channel)
 	val = PORTAXICFG_OUTTRANS_SET(val, 0xe); /* Set outstanding */
 	writel(val, mmio + PORTAXICFG);
 	readl(mmio + PORTAXICFG); /* Force a barrier */
+	/* Set the watermark threshold of the receive FIFO */
+	val = readl(mmio + PORTRANSCFG);
+	val = PORTRANSCFG_RXWM_SET(val, 0x30);
+	writel(val, mmio + PORTRANSCFG);
 }

 /**
--
1.8.2.1

[PATCH v1 2/3] ata: Fix the link down in first attempt for the APM X-Gene SoC AHCI SATA host controller driver.

[Suman Tripathi]

This patch fixes the link down issue by retry for the APM X-Gene SoC
SATA host controller driver. Due to board design issue and short margin
limitation, it is observed that once out of many thousands power cycle
test, the sata link may not link up.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---
 drivers/ata/ahci_xgene.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 03b6b0f..cc26342 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -78,6 +78,9 @@
 #define CFG_MEM_RAM_SHUTDOWN		0x00000070
 #define BLOCK_MEM_RDY			0x00000074

+/* Max retry for link down */
+#define MAX_LINK_DOWN_RETRY 3
+
 struct xgene_ahci_context {
 	struct ahci_host_priv *hpriv;
 	struct device *dev;
@@ -234,15 +237,20 @@ static int xgene_ahci_do_hardreset(struct ata_link *link,
 	u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
 	void __iomem *port_mmio = ahci_port_base(ap);
 	struct ata_taskfile tf;
+	int link_down_retry = 0;
 	int rc;
 	u32 val;

-	/* clear D2H reception area to properly wait for D2H FIS */
-	ata_tf_init(link->device, &tf);
-	tf.command = ATA_BUSY;
-	ata_tf_to_fis(&tf, 0, 0, d2h_fis);
-	rc = sata_link_hardreset(link, timing, deadline, online,
+	do {
+		/* clear D2H reception area to properly wait for D2H FIS */
+		ata_tf_init(link->device, &tf);
+		tf.command = ATA_BUSY;
+		ata_tf_to_fis(&tf, 0, 0, d2h_fis);
+		rc = sata_link_hardreset(link, timing, deadline, online,
 				 ahci_check_ready);
+		if (*online)
+			break;
+	} while (link_down_retry++ < MAX_LINK_DOWN_RETRY);

 	val = readl(port_mmio + PORT_SCR_ERR);
 	if (val & (SERR_DISPARITY | SERR_10B_8B_ERR))
--
1.8.2.1

Re: [PATCH v1 2/3] ata: Fix the link down in first attempt for the APM X-Gene SoC AHCI SATA host controller driver.

[Tejun Heo]

Hello,

On Mon, Jun 23, 2014 at 03:45:37PM +0530, Suman Tripathi wrote:
> @@ -234,15 +237,20 @@ static int xgene_ahci_do_hardreset(struct ata_link *link,
>  	u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
>  	void __iomem *port_mmio = ahci_port_base(ap);
>  	struct ata_taskfile tf;
> +	int link_down_retry = 0;
>  	int rc;
>  	u32 val;
> 
> -	/* clear D2H reception area to properly wait for D2H FIS */
> -	ata_tf_init(link->device, &tf);
> -	tf.command = ATA_BUSY;
> -	ata_tf_to_fis(&tf, 0, 0, d2h_fis);
> -	rc = sata_link_hardreset(link, timing, deadline, online,
> +	do {
> +		/* clear D2H reception area to properly wait for D2H FIS */
> +		ata_tf_init(link->device, &tf);
> +		tf.command = ATA_BUSY;
> +		ata_tf_to_fis(&tf, 0, 0, d2h_fis);
> +		rc = sata_link_hardreset(link, timing, deadline, online,
>  				 ahci_check_ready);
> +		if (*online)
> +			break;
> +	} while (link_down_retry++ < MAX_LINK_DOWN_RETRY);

Hmm... so it's retrying w/o extending deadline if the link is reported
to be offline.  This definitely needs comment explaining what's going
on.  Wouldn't it make more sense to change @timing instead rather than
retrying resets?  sata_sil24 already does something similar.  Does,
f.e., using sata_deb_timing_long for @timing make the problem go away?

Thanks.

-- 
tejun

[PATCH v1 3/3] arm64: Fix the APM X-Gene SoC SATA PHY clock DTS node csr-mask of the SATA Host Controller 1.

[Suman Tripathi]

This patch fixes the SATA PHY clock DTS node csr-mask of the SATA
Host controller 1. This patch also fixes the status of the PHY
clock node of SATA Host controller 1.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---
 arch/arm64/boot/dts/apm-storm.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
index f8c40a6..d14bcc4 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -184,9 +184,9 @@
 				reg = <0x0 0x1f21c000 0x0 0x1000>;
 				reg-names = "csr-reg";
 				clock-output-names = "sataphy1clk";
-				status = "disabled";
+				status = "ok";
 				csr-offset = <0x4>;
-				csr-mask = <0x00>;
+				csr-mask = <0x3a>;
 				enable-offset = <0x0>;
 				enable-mask = <0x06>;
 			};
--
1.8.2.1

Re: [PATCH v1 3/3] arm64: Fix the APM X-Gene SoC SATA PHY clock DTS node csr-mask of the SATA Host Controller 1.

[Sergei Shtylyov]

Hello.

On 06/23/2014 02:15 PM, Suman Tripathi wrote:

> This patch fixes the SATA PHY clock DTS node csr-mask of the SATA
> Host controller 1. This patch also fixes the status of the PHY
> clock node of SATA Host controller 1.

> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Suman Tripathi <stripathi@apm.com>
> ---
>   arch/arm64/boot/dts/apm-storm.dtsi | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

> diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
> index f8c40a6..d14bcc4 100644
> --- a/arch/arm64/boot/dts/apm-storm.dtsi
> +++ b/arch/arm64/boot/dts/apm-storm.dtsi
> @@ -184,9 +184,9 @@
>   				reg = <0x0 0x1f21c000 0x0 0x1000>;
>   				reg-names = "csr-reg";
>   				clock-output-names = "sataphy1clk";
> -				status = "disabled";
> +				status = "ok";

    You don't need the specifically set "status to "ok" unless you're 
overriding the "status" prop; "ok" is assumed AFAIK.

WBR, Sergei