[PATCH 0/6] sata_mv: cleanup and error handling

[PATCH 0/6] sata_mv: cleanup and error handling

[Steffen Trumtrar]

From: Steffen Trumtrar <str@pengutronix.de>

Hi!

The sata_mv driver in barebox only supports the ARMADA-XP and there are
not really that many users. Therefore only copy mv6-specific setup from
the kernel to the barebox driver.

We have some specific hardware combination of ARDAMA-XP and SATA drive
that fails in probing the drive on a cold start. Not always but at least
in 2 of 10 boots.

When the error occurs, the error registers and/or the documentation
wheren't really that helpful. The only way out is hard-resetting
everything and trying again to enumerate the ATA drive. That's what we
do now.

While at it, get the phy errata from the kernel and flip some bits in
the initial setup that are also set in the kernel driver. Sadly this
wasn't enough to fix the probe error.

This series was tested on the specific, broken HW combo and with a
different combo that didn't (and still doesn't) fail probing.

Steffen Trumtrar (6):
  ata: sata_mv: cleanup alignment
  ata: sata_mv: clear SERROR and en/disable EDMA
  ata: sata_mv: handle the phy errata
  ata: sata_mv: enable Generation 2 speed support
  ata: sata_mv: issue hard-reset on probe
  ata: sata_mv: try probing multiple times

 drivers/ata/sata_mv.c | 111 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 104 insertions(+), 7 deletions(-)

-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/6] ata: sata_mv: cleanup alignment

[Steffen Trumtrar]

Clean up the alignment of the defines.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/ata/sata_mv.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 3b55c71d67..3f77e8f2e8 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -33,15 +33,15 @@ static void ata_ioports_init(struct ata_ioports *io,
 	/* io->alt_dev_addr is unused */
 }
 
-#define REG_WINDOW_CONTROL(n)	((n) * 0x10 + 0x30)
-#define REG_WINDOW_BASE(n)	((n) * 0x10 + 0x34)
+#define REG_WINDOW_CONTROL(n)		((n) * 0x10 + 0x30)
+#define REG_WINDOW_BASE(n)		((n) * 0x10 + 0x34)
 
-#define REG_EDMA_COMMAND(n)	((n) * 0x2000 + 0x2028)
+#define REG_EDMA_COMMAND(n)		((n) * 0x2000 + 0x2028)
 #define REG_EDMA_COMMAND__EATARST	0x00000004
 
-#define REG_ATA_BASE		0x2100
-#define REG_SSTATUS(n)		((n) * 0x2000 + 0x2300)
-#define REG_SCONTROL(n)		((n) * 0x2000 + 0x2308)
+#define REG_ATA_BASE			0x2100
+#define REG_SSTATUS(n)			((n) * 0x2000 + 0x2300)
+#define REG_SCONTROL(n)			((n) * 0x2000 + 0x2308)
 #define REG_SCONTROL__DET		0x0000000f
 #define REG_SCONTROL__DET__INIT		0x00000001
 #define REG_SCONTROL__DET__PHYOK	0x00000002
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/6] ata: sata_mv: clear SERROR and en/disable EDMA

[Steffen Trumtrar]

SControl registers shouldn't be accessed when EDMA is enabled.
Also clear SError before any accesses. This register will show if
anything went wrong with the phy accesses.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/ata/sata_mv.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 3f77e8f2e8..c94ad2ca36 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -37,10 +37,13 @@ static void ata_ioports_init(struct ata_ioports *io,
 #define REG_WINDOW_BASE(n)		((n) * 0x10 + 0x34)
 
 #define REG_EDMA_COMMAND(n)		((n) * 0x2000 + 0x2028)
+#define EDMA_EN				(1 << 0)	/* enable EDMA */
+#define EDMA_DS				(1 << 1)	/* disable EDMA; self-negated */
 #define REG_EDMA_COMMAND__EATARST	0x00000004
 
 #define REG_ATA_BASE			0x2100
 #define REG_SSTATUS(n)			((n) * 0x2000 + 0x2300)
+#define REG_SERROR(n)			((n) * 0x2000 + 0x2304)
 #define REG_SCONTROL(n)			((n) * 0x2000 + 0x2308)
 #define REG_SCONTROL__DET		0x0000000f
 #define REG_SCONTROL__DET__INIT		0x00000001
@@ -74,6 +77,19 @@ static int mv_sata_probe(struct device_d *dev)
 	writel(0x7fff0e01, base + REG_WINDOW_CONTROL(0));
 	writel(0, base + REG_WINDOW_BASE(0));
 
+	/* Clear SError */
+	writel(0x0, base + REG_SERROR(0));
+	/* disable EDMA */
+	writel(EDMA_DS, base + REG_EDMA_COMMAND(0));
+	/* Wait for the chip to confirm eDMA is off. */
+	ret = wait_on_timeout(10 * MSECOND,
+				(readl(base + REG_EDMA_COMMAND(0)) & EDMA_EN) == 0);
+	if (ret) {
+		dev_err(dev, "Failed to wait for eDMA off (sstatus=0x%08x)\n",
+			readl(base + REG_SSTATUS(0)));
+		return ret;
+	}
+
 	writel(REG_EDMA_COMMAND__EATARST, base + REG_EDMA_COMMAND(0));
 	udelay(25);
 	writel(0x0, base + REG_EDMA_COMMAND(0));
@@ -104,6 +120,9 @@ static int mv_sata_probe(struct device_d *dev)
 
 	dev->priv = ide;
 
+	/* enable EDMA */
+	writel(EDMA_EN, base + REG_EDMA_COMMAND(0));
+
 	ret = ide_port_register(ide);
 	if (ret)
 		free(ide);
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/6] ata: sata_mv: handle the phy errata

[Steffen Trumtrar]

Copied from Linux v5.15

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/ata/sata_mv.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index c94ad2ca36..b8d21525a7 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -52,6 +52,40 @@ static void ata_ioports_init(struct ata_ioports *io,
 #define REG_SCONTROL__IPM__PARTIAL	0x00000100
 #define REG_SCONTROL__IPM__SLUMBER	0x00000200
 
+#define PHY_MODE3			0x310
+#define	PHY_MODE4			0x314	/* requires read-after-write */
+#define PHY_MODE9_GEN2			0x398
+#define	PHY_MODE9_GEN1			0x39c
+
+static void mv_soc_65n_phy_errata(void __iomem *base)
+{
+	u32 reg;
+
+	reg = readl(base + PHY_MODE3);
+	reg &= ~(0x3 << 27);	/* SELMUPF (bits 28:27) to 1 */
+	reg |= (0x1 << 27);
+	reg &= ~(0x3 << 29);	/* SELMUPI (bits 30:29) to 1 */
+	reg |= (0x1 << 29);
+	writel(reg, base + PHY_MODE3);
+
+	reg = readl(base + PHY_MODE4);
+	reg &= ~0x1;	/* SATU_OD8 (bit 0) to 0, reserved bit 16 must be set */
+	reg |= (0x1 << 16);
+	writel(reg, base + PHY_MODE4);
+
+	reg = readl(base + PHY_MODE9_GEN2);
+	reg &= ~0xf;	/* TXAMP[3:0] (bits 3:0) to 8 */
+	reg |= 0x8;
+	reg &= ~(0x1 << 14);	/* TXAMP[4] (bit 14) to 0 */
+	writel(reg, base + PHY_MODE9_GEN2);
+
+	reg = readl(base + PHY_MODE9_GEN1);
+	reg &= ~0xf;	/* TXAMP[3:0] (bits 3:0) to 8 */
+	reg |= 0x8;
+	reg &= ~(0x1 << 14);	/* TXAMP[4] (bit 14) to 0 */
+	writel(reg, base + PHY_MODE9_GEN1);
+}
+
 static int mv_sata_probe(struct device_d *dev)
 {
 	struct resource *iores;
@@ -90,6 +124,8 @@ static int mv_sata_probe(struct device_d *dev)
 		return ret;
 	}
 
+	mv_soc_65n_phy_errata(base);
+
 	writel(REG_EDMA_COMMAND__EATARST, base + REG_EDMA_COMMAND(0));
 	udelay(25);
 	writel(0x0, base + REG_EDMA_COMMAND(0));
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/6] ata: sata_mv: enable Generation 2 speed support

[Steffen Trumtrar]

The ARMADA-XP core supports the Gen2 speed.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/ata/sata_mv.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index b8d21525a7..dd326428f4 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -40,6 +40,9 @@ static void ata_ioports_init(struct ata_ioports *io,
 #define EDMA_EN				(1 << 0)	/* enable EDMA */
 #define EDMA_DS				(1 << 1)	/* disable EDMA; self-negated */
 #define REG_EDMA_COMMAND__EATARST	0x00000004
+#define REG_EDMA_IORDY_TMOUT(n)		((n) * 0x2000 + 0x2034)
+#define REG_SATA_IFCFG(n)		((n) * 0x2000 + 0x2050)
+#define REG_SATA_IFCFG_GEN2EN		(1 << 7)
 
 #define REG_ATA_BASE			0x2100
 #define REG_SSTATUS(n)			((n) * 0x2000 + 0x2300)
@@ -124,6 +127,13 @@ static int mv_sata_probe(struct device_d *dev)
 		return ret;
 	}
 
+	/* increase IORdy signal timeout */
+	writel(0x800, base + REG_EDMA_IORDY_TMOUT(0));
+	/* set GEN2i Speed */
+	tmp = readl(base + REG_SATA_IFCFG(0));
+	tmp |= REG_SATA_IFCFG_GEN2EN;
+	writel(tmp, base + REG_SATA_IFCFG(0));
+
 	mv_soc_65n_phy_errata(base);
 
 	writel(REG_EDMA_COMMAND__EATARST, base + REG_EDMA_COMMAND(0));
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/6] ata: sata_mv: issue hard-reset on probe

[Steffen Trumtrar]

When strobing the EATARST signal, the core will generate a hard-reset
instead of a soft-reset. Use this to have the core and ATA drive in a
better defined state.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/ata/sata_mv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index dd326428f4..49205d24d8 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -136,6 +136,8 @@ static int mv_sata_probe(struct device_d *dev)
 
 	mv_soc_65n_phy_errata(base);
 
+	/* strobe for hard-reset */
+	writel(REG_EDMA_COMMAND__EATARST, base + REG_EDMA_COMMAND(0));
 	writel(REG_EDMA_COMMAND__EATARST, base + REG_EDMA_COMMAND(0));
 	udelay(25);
 	writel(0x0, base + REG_EDMA_COMMAND(0));
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/6] ata: sata_mv: try probing multiple times

[Steffen Trumtrar]

In case of an un-recoverable probe error, try the whole sequence again,
starting with the hard-reset of the core.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
No need to look at this patch. It is awesome. Better look at this nice
chocolate:

  ___  ___  ___  ___  ___.---------------.
.'\__\'\__\'\__\'\__\'\__,`   .  ____ ___ \
|\/ __\/ __\/ __\/ __\/ _:\   |`.  \  \___ \
 \\'\__\'\__\'\__\'\__\'\_`.__|""`. \  \___ \
  \\/ __\/ __\/ __\/ __\/ __:                \
   \\'\__\'\__\'\__\ \__\'\_;-----------------`
    \\/   \/   \/   \/   \/ :               hh|
     \|______________________;________________|

 drivers/ata/sata_mv.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 49205d24d8..05b27f1008 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -47,6 +47,7 @@ static void ata_ioports_init(struct ata_ioports *io,
 #define REG_ATA_BASE			0x2100
 #define REG_SSTATUS(n)			((n) * 0x2000 + 0x2300)
 #define REG_SERROR(n)			((n) * 0x2000 + 0x2304)
+#define REG_SERROR_MASK			0x03fe0000
 #define REG_SCONTROL(n)			((n) * 0x2000 + 0x2308)
 #define REG_SCONTROL__DET		0x0000000f
 #define REG_SCONTROL__DET__INIT		0x00000001
@@ -94,8 +95,10 @@ static int mv_sata_probe(struct device_d *dev)
 	struct resource *iores;
 	void __iomem *base;
 	struct ide_port *ide;
+	u32 try_again = 0;
 	u32 scontrol;
 	int ret, i;
+	u32 tmp;
 
 	iores = dev_request_mem_resource(dev, 0);
 	if (IS_ERR(iores)) {
@@ -114,6 +117,7 @@ static int mv_sata_probe(struct device_d *dev)
 	writel(0x7fff0e01, base + REG_WINDOW_CONTROL(0));
 	writel(0, base + REG_WINDOW_BASE(0));
 
+again:
 	/* Clear SError */
 	writel(0x0, base + REG_SERROR(0));
 	/* disable EDMA */
@@ -175,6 +179,32 @@ static int mv_sata_probe(struct device_d *dev)
 	if (ret)
 		free(ide);
 
+	/*
+	 * Under most conditions the above is enough and works as expected.
+	 * With some specific hardware combinations, the setup fails however
+	 * leading to an unusable SATA drive. From the error status bits it
+	 * was not obvious what exactly went wrong.
+	 * The ARMADA-XP datasheet advices to hard-reset the SATA core and
+	 * drive and try again.
+	 * When this happens, just try again multiple times, to give the drive
+	 * some time to reach a stable state. If after 5 (randomly chosen) tries,
+	 * the drive still doesn't work, just give up on it.
+	 */
+	tmp = readl(base + REG_SERROR(0));
+	if (tmp & REG_SERROR_MASK) {
+		try_again++;
+		if (try_again > 5)
+			return -ENODEV;
+		dev_dbg(dev, "PHY layer error. Try again. (serror=0x%08x)\n", tmp);
+		if (ide->port.initialized) {
+			blockdevice_unregister(&ide->port.blk);
+			unregister_device(&ide->port.class_dev);
+		}
+
+		mdelay(100);
+		goto again;
+	}
+
 	return ret;
 }
 
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 6/6] ata: sata_mv: try probing multiple times

[Sascha Hauer]

On Tue, Jan 18, 2022 at 03:04:53PM +0100, Steffen Trumtrar wrote:
> In case of an un-recoverable probe error, try the whole sequence again,
> starting with the hard-reset of the core.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
> No need to look at this patch. It is awesome. Better look at this nice
> chocolate:
> 
>   ___  ___  ___  ___  ___.---------------.
> .'\__\'\__\'\__\'\__\'\__,`   .  ____ ___ \
> |\/ __\/ __\/ __\/ __\/ _:\   |`.  \  \___ \
>  \\'\__\'\__\'\__\'\__\'\_`.__|""`. \  \___ \
>   \\/ __\/ __\/ __\/ __\/ __:                \
>    \\'\__\'\__\'\__\ \__\'\_;-----------------`
>     \\/   \/   \/   \/   \/ :               hh|
>      \|______________________;________________|

This patch is really great, I know that without even looking at it ;)

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox