Promise ATA engines need to be reset when errors are detected.
That's currently done for errors detected by sata_promise itself,
but it's not done for errors like timeouts detected outside of
the low-level driver.
The effect of this omission is that a timeout tends to result
in a sequence of failed COMRESETs after which libata EH gives
up and disables the port. At that point the port's ATA engine
hangs and even reloading the driver will not resume it.
To fix this, make sata_promise override ->hardreset with code
which calls pdc_reset_port() on the port in question before
calling libata's hardreset. libata ignores its own hardreset on
PATA ports, so we need a special ->hardreset for PATA ports
that doesn't call into libata after the pdc_reset_port().
Signed-off-by: Mikael Pettersson <[email protected]>
---
This should also go into the 2.6.27 and 2.6.26 stable branches.
In older branches libata EH is different and I'm seeing different
behaviour on timeouts, so I don't think this should go in as-is
in 2.6.25 and older.
drivers/ata/sata_promise.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff -rupN linux-2.6.28-rc2/drivers/ata/sata_promise.c linux-2.6.28-rc2.sata_promise-proper-hardreset/drivers/ata/sata_promise.c
--- linux-2.6.28-rc2/drivers/ata/sata_promise.c 2008-10-26 21:35:33.000000000 +0100
+++ linux-2.6.28-rc2.sata_promise-proper-hardreset/drivers/ata/sata_promise.c 2008-10-26 23:46:03.000000000 +0100
@@ -153,6 +153,10 @@ static void pdc_freeze(struct ata_port *
static void pdc_sata_freeze(struct ata_port *ap);
static void pdc_thaw(struct ata_port *ap);
static void pdc_sata_thaw(struct ata_port *ap);
+static int pdc_pata_hardreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline);
+static int pdc_sata_hardreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline);
static void pdc_error_handler(struct ata_port *ap);
static void pdc_post_internal_cmd(struct ata_queued_cmd *qc);
static int pdc_pata_cable_detect(struct ata_port *ap);
@@ -186,6 +190,7 @@ static struct ata_port_operations pdc_sa
.scr_read = pdc_sata_scr_read,
.scr_write = pdc_sata_scr_write,
.port_start = pdc_sata_port_start,
+ .hardreset = pdc_sata_hardreset,
};
/* First-generation chips need a more restrictive ->check_atapi_dma op */
@@ -200,6 +205,7 @@ static struct ata_port_operations pdc_pa
.freeze = pdc_freeze,
.thaw = pdc_thaw,
.port_start = pdc_common_port_start,
+ .hardreset = pdc_pata_hardreset,
};
static const struct ata_port_info pdc_port_info[] = {
@@ -693,6 +699,20 @@ static void pdc_sata_thaw(struct ata_por
readl(host_mmio + hotplug_offset); /* flush */
}
+static int pdc_pata_hardreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ pdc_reset_port(link->ap);
+ return 0;
+}
+
+static int pdc_sata_hardreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ pdc_reset_port(link->ap);
+ return sata_sff_hardreset(link, class, deadline);
+}
+
static void pdc_error_handler(struct ata_port *ap)
{
if (!(ap->pflags & ATA_PFLAG_FROZEN))
Hello, Mikael.
Mikael Pettersson wrote:
> +static int pdc_pata_hardreset(struct ata_link *link, unsigned int *class,
> + unsigned long deadline)
> +{
> + pdc_reset_port(link->ap);
> + return 0;
> +}
You either need to return -EAGAIN here to ask for follow-up SRST or
override softreset. I think pdc_reset_port() doesn't actually reset the
ATA channel, right? In that case, the latter would be the correct thing
to do.
Thanks.
--
tejun
Tejun Heo writes:
> Hello, Mikael.
>
> Mikael Pettersson wrote:
> > +static int pdc_pata_hardreset(struct ata_link *link, unsigned int *class,
> > + unsigned long deadline)
> > +{
> > + pdc_reset_port(link->ap);
> > + return 0;
> > +}
>
> You either need to return -EAGAIN here to ask for follow-up SRST or
> override softreset. I think pdc_reset_port() doesn't actually reset the
> ATA channel, right? In that case, the latter would be the correct thing
> to do.
(ponders..) ah yes, pata normally skips hardreset so this would
make it skip the needed softreset. pdc_reset_port() just reinits
the ATA engine, nothing else should be affected.
I'll update the patch to make pata override ->softreset instead.
Thanks.
/Mikael