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
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
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
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
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
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
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
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