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

[PATCH v4 0/4]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.

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.

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

Suman Tripathi (4):
  ata: Fix the watermark threshold for the APM X-Gene SATA host
    controller driver.
  arm64: Remove the clock and PHY reference from the APM X-Gene SoC AHCI
    SATA Host controller dts node.
  ata: Use correct OOB tunning parameters for APM X-Gene SoC AHCI SATA
    Host controller driver.
  ata: Removing NCQ support from the APM X-Gene SoC AHCI SATA Host
    Controller driver.

 arch/arm64/boot/dts/apm-storm.dtsi | 92 --------------------------------------
 drivers/ata/ahci_xgene.c           | 17 ++++---
 2 files changed, 12 insertions(+), 97 deletions(-)

--
1.8.2.1

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

[Suman Tripathi]

As per SATA IO specification, when Host sends HOLD, the device takes
about 20DW latency to reply to HOLDA. In some case, device doesn't
response to HOLDA over 20DW and causes FIFO goes into over flow
condition. Due to this condition, device enumerations fails with
those devices. This patch adjust the watermark FIFO by increasing
the FIFO depth from 0x16(default) to 0x30 to address this issue.

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 v4 2/4] arm64: Remove the clock and PHY reference from the APM X-Gene SoC AHCI SATA Host controller dts node.

[Suman Tripathi]

This patch removes all clocks and PHY references from the APM X-Gene
SoC AHCI SATA host controller and PHY DTS nodes. The clock and PHY
are no longer needed as they are handled by the firmware. By removing
only the reference is not enough as any un-used clock entry will get
disabled by the clock framework.

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

diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
index f8c40a6..fa1a57f 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -177,86 +177,6 @@
 				clock-output-names = "eth8clk";
 			};

-			sataphy1clk: sataphy1clk@1f21c000 {
-				compatible = "apm,xgene-device-clock";
-				#clock-cells = <1>;
-				clocks = <&socplldiv2 0>;
-				reg = <0x0 0x1f21c000 0x0 0x1000>;
-				reg-names = "csr-reg";
-				clock-output-names = "sataphy1clk";
-				status = "disabled";
-				csr-offset = <0x4>;
-				csr-mask = <0x00>;
-				enable-offset = <0x0>;
-				enable-mask = <0x06>;
-			};
-
-			sataphy2clk: sataphy1clk@1f22c000 {
-				compatible = "apm,xgene-device-clock";
-				#clock-cells = <1>;
-				clocks = <&socplldiv2 0>;
-				reg = <0x0 0x1f22c000 0x0 0x1000>;
-				reg-names = "csr-reg";
-				clock-output-names = "sataphy2clk";
-				status = "ok";
-				csr-offset = <0x4>;
-				csr-mask = <0x3a>;
-				enable-offset = <0x0>;
-				enable-mask = <0x06>;
-			};
-
-			sataphy3clk: sataphy1clk@1f23c000 {
-				compatible = "apm,xgene-device-clock";
-				#clock-cells = <1>;
-				clocks = <&socplldiv2 0>;
-				reg = <0x0 0x1f23c000 0x0 0x1000>;
-				reg-names = "csr-reg";
-				clock-output-names = "sataphy3clk";
-				status = "ok";
-				csr-offset = <0x4>;
-				csr-mask = <0x3a>;
-				enable-offset = <0x0>;
-				enable-mask = <0x06>;
-			};
-
-			sata01clk: sata01clk@1f21c000 {
-				compatible = "apm,xgene-device-clock";
-				#clock-cells = <1>;
-				clocks = <&socplldiv2 0>;
-				reg = <0x0 0x1f21c000 0x0 0x1000>;
-				reg-names = "csr-reg";
-				clock-output-names = "sata01clk";
-				csr-offset = <0x4>;
-				csr-mask = <0x05>;
-				enable-offset = <0x0>;
-				enable-mask = <0x39>;
-			};
-
-			sata23clk: sata23clk@1f22c000 {
-				compatible = "apm,xgene-device-clock";
-				#clock-cells = <1>;
-				clocks = <&socplldiv2 0>;
-				reg = <0x0 0x1f22c000 0x0 0x1000>;
-				reg-names = "csr-reg";
-				clock-output-names = "sata23clk";
-				csr-offset = <0x4>;
-				csr-mask = <0x05>;
-				enable-offset = <0x0>;
-				enable-mask = <0x39>;
-			};
-
-			sata45clk: sata45clk@1f23c000 {
-				compatible = "apm,xgene-device-clock";
-				#clock-cells = <1>;
-				clocks = <&socplldiv2 0>;
-				reg = <0x0 0x1f23c000 0x0 0x1000>;
-				reg-names = "csr-reg";
-				clock-output-names = "sata45clk";
-				csr-offset = <0x4>;
-				csr-mask = <0x05>;
-				enable-offset = <0x0>;
-				enable-mask = <0x39>;
-			};
 		};

 		serial0: serial@1c020000 {
@@ -273,7 +193,6 @@
 			compatible = "apm,xgene-phy";
 			reg = <0x0 0x1f21a000 0x0 0x100>;
 			#phy-cells = <1>;
-			clocks = <&sataphy1clk 0>;
 			status = "disabled";
 			apm,tx-boost-gain = <30 30 30 30 30 30>;
 			apm,tx-eye-tuning = <2 10 10 2 10 10>;
@@ -283,7 +202,6 @@
 			compatible = "apm,xgene-phy";
 			reg = <0x0 0x1f22a000 0x0 0x100>;
 			#phy-cells = <1>;
-			clocks = <&sataphy2clk 0>;
 			status = "ok";
 			apm,tx-boost-gain = <30 30 30 30 30 30>;
 			apm,tx-eye-tuning = <1 10 10 2 10 10>;
@@ -293,7 +211,6 @@
 			compatible = "apm,xgene-phy";
 			reg = <0x0 0x1f23a000 0x0 0x100>;
 			#phy-cells = <1>;
-			clocks = <&sataphy3clk 0>;
 			status = "ok";
 			apm,tx-boost-gain = <31 31 31 31 31 31>;
 			apm,tx-eye-tuning = <2 10 10 2 10 10>;
@@ -309,9 +226,6 @@
 			interrupts = <0x0 0x86 0x4>;
 			dma-coherent;
 			status = "disabled";
-			clocks = <&sata01clk 0>;
-			phys = <&phy1 0>;
-			phy-names = "sata-phy";
 		};

 		sata2: sata@1a400000 {
@@ -324,9 +238,6 @@
 			interrupts = <0x0 0x87 0x4>;
 			dma-coherent;
 			status = "ok";
-			clocks = <&sata23clk 0>;
-			phys = <&phy2 0>;
-			phy-names = "sata-phy";
 		};

 		sata3: sata@1a800000 {
@@ -338,9 +249,6 @@
 			interrupts = <0x0 0x88 0x4>;
 			dma-coherent;
 			status = "ok";
-			clocks = <&sata45clk 0>;
-			phys = <&phy3 0>;
-			phy-names = "sata-phy";
 		};
 	};
 };
--
1.8.2.1

Re: [PATCH v4 2/4] arm64: Remove the clock and PHY reference from the APM X-Gene SoC AHCI SATA Host controller dts node.

[Arnd Bergmann]

On Tuesday 29 July 2014 12:24:50 Suman Tripathi wrote:
> This patch removes all clocks and PHY references from the APM X-Gene
> SoC AHCI SATA host controller and PHY DTS nodes. The clock and PHY
> are no longer needed as they are handled by the firmware. By removing
> only the reference is not enough as any un-used clock entry will get
> disabled by the clock framework.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Suman Tripathi <stripathi@apm.com>
> 

This will break booting on older firmware versions I guess, and
it does not confirm to the binding that lists the phy and clock
properties as "required".

It doesn't sound like a good idea to apply it.

	Arnd

[PATCH v4 3/4] ata: Use correct OOB tunning parameters for APM X-Gene SoC AHCI SATA Host controller driver.

[Suman Tripathi]

APM X-Gene SoC AHCI SATA Host controller driver requires
some correction of Phy Control OOB timing for the
COMINIT/COMWAKE parameters to correctly interoperate with
different kinds of disks.

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

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 03b6b0f..1d48371 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -163,11 +163,11 @@ static void xgene_ahci_set_phy_cfg(struct xgene_ahci_context *ctx, int channel)
 	/* Disable fix rate */
 	writel(0x0001fffe, mmio + PORTPHY1CFG);
 	readl(mmio + PORTPHY1CFG); /* Force a barrier */
-	writel(0x5018461c, mmio + PORTPHY2CFG);
+	writel(0x28183219, mmio + PORTPHY2CFG);
 	readl(mmio + PORTPHY2CFG); /* Force a barrier */
-	writel(0x1c081907, mmio + PORTPHY3CFG);
+	writel(0x13081008, mmio + PORTPHY3CFG);
 	readl(mmio + PORTPHY3CFG); /* Force a barrier */
-	writel(0x1c080815, mmio + PORTPHY4CFG);
+	writel(0x00480815, mmio + PORTPHY4CFG);
 	readl(mmio + PORTPHY4CFG); /* Force a barrier */
 	/* Set window negotiation */
 	val = readl(mmio + PORTPHY5CFG);
--
1.8.2.1

[PATCH v4 4/4] ata: Removing NCQ support from the APM X-Gene SoC AHCI SATA Host Controller driver.

[Suman Tripathi]

This patch removes the NCQ support from the APM X-Gene SoC AHCI
Host Controller driver as it doesn't support it.

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

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 1d48371..4496ddf 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -310,8 +310,8 @@ static struct ata_port_operations xgene_ahci_ops = {
 };

 static const struct ata_port_info xgene_ahci_port_info = {
-	AHCI_HFLAGS(AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ),
-	.flags = AHCI_FLAG_COMMON | ATA_FLAG_NCQ,
+	AHCI_HFLAGS(AHCI_HFLAG_NO_PMP | AHCI_HFLAG_NO_NCQ),
+	.flags = AHCI_FLAG_COMMON,
 	.pio_mask = ATA_PIO4,
 	.udma_mask = ATA_UDMA6,
 	.port_ops = &xgene_ahci_ops,
--
1.8.2.1

Re: [PATCH v4 0/4]ata: Fixes related to APM X-Gene SATA host controller driver.

[Tejun Heo]

On Tue, Jul 29, 2014 at 12:24:48PM +0530, Suman Tripathi wrote:
> 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.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Suman Tripathi <stripathi@apm.com>

Applied 1 and 3 to libata/for-3.17.  4 doesn't apply.  Also, please
prefix the patches with "ahci_xgene: " from now on.

Thanks.

-- 
tejun

Re: [PATCH v4 0/4]ata: Fixes related to APM X-Gene SATA host controller driver.

[Tejun Heo]

On Tue, Jul 29, 2014 at 08:05:45PM +0530, Suman Tripathi wrote:
> Hi,
> 
> Applied 1 and 3 to libata/for-3.17.  4 doesn't apply.  Also, please
> prefix the patches with "ahci_xgene: " from now on.
> 
> [suman] : You mean the "Remove NCQ patch" is not applied. Any reason for
> that ?

I meant that the patch doesn't apply to libata/for-3.17.  Please
always generate patches against the current devel branch.

Thanks.

-- 
tejun