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

[PATCH v7 0/3] ahci_xgene: 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.

v2 Change:
   1. Drop the Link down retry patch from this patch set.

v4 Change:
   1. Drop the patch to fix the csr-mask in dts for PHY clock
     node of SATA Host Controller 1.
   2. Add the patch to correct the OOB tunning parameters for
     the COMINIT/COMWAKE parameters.
   3. Add the patch to remove the NCQ support from the APM
     X-Gene AHCI SATA Host controller driver.
   4. Add the patch to remove the clock and PHY reference nodes
     from the APM X-Gene Host controller dts node.

v5 Change :
   1. All the patches are based on 3.16.0-rc6/for-3.17 kernel.
   2. Drop the patch to remove the clock and PHY reference nodes
     from the APM X-Gene Host controller dts node as it breaks
     with old firmware.
   3. Add the patch to skip phy and clock initialisation if
     already done in the firmware.
   4. Add the patch to fix the csr-mask in dts for PHY clock
     node of SATA Host Controller 1.
   5. Add the patch to remove the NCQ support from the APM
     X-Gene AHCI SATA Host controller driver based on 3.16.0-rc6/
     for-3.17 kernel.
   6. Drop the patch to correct the OOB tunning parameters for
     the COMINIT/COMWAKE parameters as it is already applied to
     3.16/for-3.17 branch by the maintainer.
   7. Drop the patch to fix the watermark threshold as it is
     already applied to 3.16/for-3.17 branch by the maintainer.

v6 change :
   1. Drop the patch to skip phy and clock initialisation if
     already done in the firmware.
   2. Drop the patch to fix the csr-mask in dts for PHY clock
     node of SATA Host Controller 1.
   3. Add the remove the clock and PHY node patch as it
     fixes the dropped patches together. This patch works with
     old and new firmware as well.

v7 change :
   1. Drop the patch to remove the clock and PHY reference nodes
     from the APM X-Gene Host controller dts node as it breaks
     with old firmware if sata not initialised from the firmware.
   2. Add the patch to skip phy and clock initialisation if
     already done in the firmware.
   3. Add the patch to fix the csr-mask in dts for PHY clock
     node of SATA Host Controller 1.
   4. Add the Link down retry patch with a proper comments and
     explanation.
   5. Drop the patch to remove NCQ as it is applied to 3.16/for-3.17.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Suman Tripathi <stripathi@apm.com>
---

Suman Tripathi (3):
  arm64: Fix the csr-mask for APM X-Gene SoC AHCI SATA PHY clock DTS
    node.
  ahci_xgene: Skip the PHY and clock initialization if already
    configured by the firmware.
  ahci_xgene: Fix the link down in first attempt for the APM X-Gene SoC
    AHCI SATA host controller driver.

 arch/arm64/boot/dts/apm-storm.dtsi |  5 +----
 drivers/ata/ahci_xgene.c           | 42 +++++++++++++++++++++++++++++++-------
 2 files changed, 36 insertions(+), 11 deletions(-)

--
1.8.2.1

[PATCH v7 1/3] arm64: Fix the csr-mask for APM X-Gene SoC AHCI SATA PHY clock DTS node.

[Suman Tripathi]

The value of the csr-mask of the SATA PHY clock DTS node has a
wrong value resulting a kernel panic as the clock/reset is not
proper for the PHY of the SATA host controller 1. This patch
fixes the correct csr-mask value of the SATA PHY clock DTS node
for the SATA Host controller 1.

As the 'ok' is the default status of a device tree node, this patch
removes the status of the PHY clock node of SATA Host Controller 1.
The status of the clock node is handled from the firmware based on
the controller enabled/disabled by the user.

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

diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
index 40aa96c..1bdaeda 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -184,9 +184,8 @@
 				reg = <0x0 0x1f21c000 0x0 0x1000>;
 				reg-names = "csr-reg";
 				clock-output-names = "sataphy1clk";
-				status = "disabled";
 				csr-offset = <0x4>;
-				csr-mask = <0x00>;
+				csr-mask = <0x3a>;
 				enable-offset = <0x0>;
 				enable-mask = <0x06>;
 			};
@@ -198,7 +197,6 @@
 				reg = <0x0 0x1f22c000 0x0 0x1000>;
 				reg-names = "csr-reg";
 				clock-output-names = "sataphy2clk";
-				status = "ok";
 				csr-offset = <0x4>;
 				csr-mask = <0x3a>;
 				enable-offset = <0x0>;
@@ -212,7 +210,6 @@
 				reg = <0x0 0x1f23c000 0x0 0x1000>;
 				reg-names = "csr-reg";
 				clock-output-names = "sataphy3clk";
-				status = "ok";
 				csr-offset = <0x4>;
 				csr-mask = <0x3a>;
 				enable-offset = <0x0>;
--
1.8.2.1

[PATCH v7 2/3] ahci_xgene: Skip the PHY and clock initialization if already configured by the firmware.

[Suman Tripathi]

This patch implements the feature to skip the PHY and clock
initialization if it is already configured by the firmware.

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

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index f416495..0a87f2e 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -145,6 +145,16 @@ static unsigned int xgene_ahci_qc_issue(struct ata_queued_cmd *qc)
 	return rc;
 }

+static int xgene_ahci_is_memram_inited(struct xgene_ahci_context *ctx)
+{
+	void __iomem *diagcsr = ctx->csr_diag;
+
+	if (readl(diagcsr + CFG_MEM_RAM_SHUTDOWN) == 0 &&
+	    readl(diagcsr + BLOCK_MEM_RDY) == 0xFFFFFFFF)
+		return 1;
+	return 0;
+}
+
 /**
  * xgene_ahci_read_id - Read ID data from the specified device
  * @dev: device
@@ -468,6 +478,11 @@ static int xgene_ahci_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}

+	if (xgene_ahci_is_memram_inited(ctx)) {
+		dev_info(dev, "skip clock and PHY initialization\n");
+		goto skip_clk_phy;
+	}
+
 	/* Due to errata, HW requires full toggle transition */
 	rc = ahci_platform_enable_clks(hpriv);
 	if (rc)
@@ -481,6 +496,8 @@ static int xgene_ahci_probe(struct platform_device *pdev)
 	/* Configure the host controller */
 	xgene_ahci_hw_init(hpriv);

+skip_clk_phy:
+
 	hflags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_NO_NCQ;

 	rc = ahci_platform_init_host(pdev, hpriv, &xgene_ahci_port_info,
--
1.8.2.1

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

[Suman Tripathi]

The link down issue in first attempt happens due to 2 H/W errata below:

1. Due to HW errata, during speed negotiation, sometimes controller
is not able to detect ALIGN at GEN3(6Gbps) within 54.6us results in
a timeout. This issue can be recovered by issuing a COMRESET again.

2. Due to HW errata, although ALIGH detection is successfull, due to
8b/10b and disparity BERR, sometimes the signature from the drive is
not received successfully by the Host controller. Due to this the
communication with the host and drive is not established due to
locking of CDR(clock and data recovery) circuit. This issue can be
recovered by issuing a COMRESET again.

This patch fixes the above issues by retrying the COMRESET with a
maximum attempts of 3.
---
 drivers/ata/ahci_xgene.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 0a87f2e..294c9a8 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;
@@ -277,15 +280,23 @@ 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,
+	u32 val, sstatus;
+
+	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;
+
+		sata_scr_read(link, SCR_STATUS, &sstatus);
+	} while (link_down_retry++ < MAX_LINK_DOWN_RETRY &&
+		 (sstatus & 0xff) == 0x1);

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

Re: [PATCH v7 2/3] ahci_xgene: Skip the PHY and clock initialization if already configured by the firmware.

[Tejun Heo]

On Tue, Aug 19, 2014 at 12:01:50PM +0530, Suman Tripathi wrote:
> This patch implements the feature to skip the PHY and clock
> initialization if it is already configured by the firmware.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Suman Tripathi <stripathi@apm.com>
...
> +static int xgene_ahci_is_memram_inited(struct xgene_ahci_context *ctx)
> +{
> +	void __iomem *diagcsr = ctx->csr_diag;
> +
> +	if (readl(diagcsr + CFG_MEM_RAM_SHUTDOWN) == 0 &&
> +	    readl(diagcsr + BLOCK_MEM_RDY) == 0xFFFFFFFF)
> +		return 1;
> +	return 0;
> +}

Please make it return bool.

Thanks.

-- 
tejun

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

[Tejun Heo]

On Tue, Aug 19, 2014 at 12:01:51PM +0530, Suman Tripathi wrote:
> The link down issue in first attempt happens due to 2 H/W errata below:
> 
> 1. Due to HW errata, during speed negotiation, sometimes controller
> is not able to detect ALIGN at GEN3(6Gbps) within 54.6us results in
> a timeout. This issue can be recovered by issuing a COMRESET again.
> 
> 2. Due to HW errata, although ALIGH detection is successfull, due to
> 8b/10b and disparity BERR, sometimes the signature from the drive is
> not received successfully by the Host controller. Due to this the
> communication with the host and drive is not established due to
> locking of CDR(clock and data recovery) circuit. This issue can be
> recovered by issuing a COMRESET again.
> 
> This patch fixes the above issues by retrying the COMRESET with a
> maximum attempts of 3.

It's kinda nasty but if it's necessary.  That said, can you please
update the comment so that it actually matches the code?  Also,
Wouldn't it be better to check how the reset failed before retrying?

Thanks.

-- 
tejun

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

[Tejun Heo]

On Thu, Aug 21, 2014 at 01:48:00PM +0530, Suman Tripathi wrote:
> [suman] : The problem is COMRESET didn't failed. I meant the hardreset is
> successful (return 0) but the device is not detected even if device is
> present due to speed negotiation failure. For that reason I check for the
> Pxstatus and retried.

Alright, please update the comment to explain what's going on.

Thanks.

-- 
tejun