[PATCH v9 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.

v8 change :
1. Adding proper comments for link down retry patch.
2. changing the return type to bool for skip phy patch.

v9 change:
1. Fixing the code to match the comment for link down
retry patch.

Signed-off-by: Loc Ho <***@apm.com>
Signed-off-by: Suman Tripathi <***@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 | 63 +++++++++++++++++++++++++++++---------
2 files changed, 50 insertions(+), 18 deletions(-)

--
1.8.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[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 <***@apm.com>
Signed-off-by: Suman Tripathi <***@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

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Suman Tripathi]

Due to HW errata the APM X-Gene AHCI SATA host controller reports link
down even if the device presence is detected. This issue is due to speed
negotiation failure. This patch implements the algorithm to retry the
COMRESET if PxSTAT register reports device presence detected but
PHY communication not established. The maximum retry attempts are 3.

This patch also fixes the code to match the algorithm for the printing
a warning message if the disparity error still exists after link up.

Signed-off-by: Loc Ho <***@apm.com>
Signed-off-by: Suman Tripathi <***@apm.com>
---
drivers/ata/ahci_xgene.c | 48 ++++++++++++++++++++++++++++++++++--------------
1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index c721887..40d0a76 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;
@@ -237,8 +240,11 @@ static void xgene_ahci_set_phy_cfg(struct xgene_ahci_context *ctx, int channel)
* and Gen1 (1.5Gbps). Otherwise during long IO stress test, the PHY will
* report disparity error and etc. In addition, during COMRESET, there can
* be error reported in the register PORT_SCR_ERR. For SERR_DISPARITY and
- * SERR_10B_8B_ERR, the PHY receiver line must be reseted. The following
- * algorithm is followed to proper configure the hardware PHY during COMRESET:
+ * SERR_10B_8B_ERR, the PHY receiver line must be reseted. Also during long
+ * reboot cycle regression, sometimes the PHY reports link down even if the
+ * device is present because of speed negotiation failure. so need to retry
+ * the COMRESET to get the link up. The following algorithm is followed to
+ * proper configure the hardware PHY during COMRESET:
*
* Alg Part 1:
* 1. Start the PHY at Gen3 speed (default setting)
@@ -254,9 +260,15 @@ static void xgene_ahci_set_phy_cfg(struct xgene_ahci_context *ctx, int channel)
* Alg Part 2:
* 1. On link up, if there are any SERR_DISPARITY and SERR_10B_8B_ERR error
* reported in the register PORT_SCR_ERR, then reset the PHY receiver line
- * 2. Go to Alg Part 3
+ * 2. Go to Alg Part 4
*
* Alg Part 3:
+ * 1. Check the PORT_SCR_STAT to see whether device presence detected but PHY
+ * communication establishment failed and maximum link down attempts are
+ * less than Max attempts 3 then goto Alg Part 1.
+ * 2. Go to Alg Part 4.
+ *
+ * Alg Part 4:
* 1. Clear any pending from register PORT_SCR_ERR.
*
* NOTE: For the initial version, we will NOT support Gen1/Gen2. In addition
@@ -275,19 +287,27 @@ 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);
-
- val = readl(port_mmio + PORT_SCR_ERR);
- if (val & (SERR_DISPARITY | SERR_10B_8B_ERR))
- dev_warn(ctx->dev, "link has error\n");
+ if (*online) {
+ val = readl(port_mmio + PORT_SCR_ERR);
+ if (val & (SERR_DISPARITY | SERR_10B_8B_ERR))
+ dev_warn(ctx->dev, "link has error\n");
+ break;
+ }
+
+ sata_scr_read(link, SCR_STATUS, &sstatus);
+ } while (link_down_retry++ < MAX_LINK_DOWN_RETRY &&
+ (sstatus & 0xff) == 0x1);

/* clear all errors if any pending */
val = readl(port_mmio + PORT_SCR_ERR);
--
1.8.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tejun Heo]

Post by Suman Tripathi
Due to HW errata the APM X-Gene AHCI SATA host controller reports link
down even if the device presence is detected. This issue is due to speed
negotiation failure. This patch implements the algorithm to retry the
COMRESET if PxSTAT register reports device presence detected but
PHY communication not established. The maximum retry attempts are 3.
This patch also fixes the code to match the algorithm for the printing
a warning message if the disparity error still exists after link up.

Applied to libata/for-3.17-fixes.

Thanks.

[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 <***@apm.com>
Signed-off-by: Suman Tripathi <***@apm.com>
---
drivers/ata/ahci_xgene.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

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

+static bool xgene_ahci_is_memram_inited(struct xgene_ahci_context *ctx)
+{
+ void __iomem *diagcsr = ctx->csr_diag;
+
+ return (readl(diagcsr + CFG_MEM_RAM_SHUTDOWN) == 0 &&
+ readl(diagcsr + BLOCK_MEM_RDY) == 0xFFFFFFFF);
+}
+
/**
* xgene_ahci_read_id - Read ID data from the specified device
* @dev: device
@@ -468,6 +476,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 +494,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

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tejun Heo]

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

Applied to libata/for-3.17-fixes.

Thanks.