On Fri, May 26 2006, Linus Torvalds wrote:
>
>
> On Fri, 26 May 2006, Mark Lord wrote:
> >
> > Well, this problem has been with us all for a year now,
> > and at this point it impacts practically *every* new "centrino"
> > notebook out there.
> >
> > We have a very simple workaround (previous post) that addresses it
> > for 2.6.17, and it's about damn time it got fixed.
> >
> > If there's a better solution for *2.6.17*, then *please* post it.
> > Otherwise, we have a fix. Maybe Linus or Andrew should just apply it?
>
> I'm definitely in the "at some point, protesting a patch that works
> becomes an untenably position to take, no matter _how_ ugly the patch is"
> camp.
>
> If the people who complain that it is ugly cannot come up with an
> alternate solution that works and isn't ugly, at some point the "ugly"
> complaint just becomes totally pointless.
If that wasn't the case, then we wouldn't even have basic sata suspend
support in the Linus kernels right now... So agreed.
> Of course, I'm not on linux-ide, and I didn't see this particular
> discussion from the start (or even the alledged simple workaround in the
> "previous post"), but can people please fill me in? And if the choice is
> not between "ugly" vs "pretty", but between "ugly" vs "nonworking", I
> think we know what the answer should be.
There was no real discussion on this issue yet. I think we all agree
that the functionality of the patch (waiting for BSY clear on resume) is
the right thing to do. This posted patch moved SCSI stuff into ata_piix,
which isn't really very nice. Jeff wants to do it from the pci resume,
which just seems wrong to me since it's a device (disk) condition not a
"host" condition. FWIW, here's what I had in mind as suggested in the
original reply. Not tested at all for functionality, but it compiles.
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index 6dc8814..103afc3 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -151,6 +151,7 @@ static int piix_pata_probe_reset(struct
static int piix_sata_probe_reset(struct ata_port *ap, unsigned int *classes);
static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev);
static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev);
+static int piix_port_resume(struct ata_port *ap);
static unsigned int in_module_init = 1;
@@ -250,6 +251,7 @@ static const struct ata_port_operations
.port_start = ata_port_start,
.port_stop = ata_port_stop,
+ .port_resume = piix_port_resume,
.host_stop = ata_host_stop,
};
@@ -738,6 +740,16 @@ static int piix_disable_ahci(struct pci_
return rc;
}
+static int piix_port_resume(struct ata_port *ap)
+{
+ u8 status = ata_busy_wait(ap, ATA_BUSY, 200000);
+
+ if (status & ATA_BUSY)
+ return 1;
+
+ return 0;
+}
+
/**
* piix_check_450nx_errata - Check for problem 450NX setup
* @ata_dev: the PCI device to check
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index fa476e7..ae7fac1 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -4298,6 +4298,8 @@ int ata_device_resume(struct ata_port *a
{
if (ap->flags & ATA_FLAG_SUSPENDED) {
ap->flags &= ~ATA_FLAG_SUSPENDED;
+ if (ap->ops->port_resume)
+ ap->ops->port_resume(ap);
ata_set_mode(ap);
}
if (!ata_dev_present(dev))
diff --git a/include/linux/libata.h b/include/linux/libata.h
index b80d2e7..0be5d02 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -461,6 +461,7 @@ struct ata_port_operations {
int (*port_start) (struct ata_port *ap);
void (*port_stop) (struct ata_port *ap);
+ int (*port_resume) (struct ata_port *ap);
void (*host_stop) (struct ata_host_set *host_set);
--
Jens Axboe
-
: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
