* [PATCH] libata: always use polling IDENTIFY
@ 2006-12-03 10:30 Tejun Heo
2006-12-03 11:14 ` Jeff Garzik
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tejun Heo @ 2006-12-03 10:30 UTC (permalink / raw)
To: Jeff Garzik, Alan Cox, linux-ide
libata switched to IRQ-driven IDENTIFY when IRQ-driven PIO was
introduced. This has caused a lot of problems including device
misdetection and phantom device.
ATA_FLAG_DETECT_POLLING was added recently to selectively use polling
IDENTIFY on problemetic drivers but many controllers and devices are
affected by this problem and trying to adding ATA_FLAG_DETECT_POLLING
for each such case is diffcult and not very rewarding.
This patch makes libata always use polling IDENTIFY. This is
consistent with libata's original behavior and drivers/ide's behavior.
Signed-off-by: Tejun Heo <htejun@gmail.com>
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 788a269..a2d84f7 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -105,9 +105,8 @@ enum {
PIIX_FLAG_AHCI = (1 << 27), /* AHCI possible */
PIIX_FLAG_CHECKINTR = (1 << 28), /* make sure PCI INTx enabled */
- PIIX_PATA_FLAGS = ATA_FLAG_SLAVE_POSS | ATA_FLAG_DETECT_POLLING,
- PIIX_SATA_FLAGS = ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR |
- ATA_FLAG_DETECT_POLLING,
+ PIIX_PATA_FLAGS = ATA_FLAG_SLAVE_POSS,
+ PIIX_SATA_FLAGS = ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR,
/* combined mode. if set, PATA is channel 0.
* if clear, PATA is channel 1.
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 3fd7c79..cc0bb59 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1314,16 +1314,12 @@ int ata_dev_read_id(struct ata_device *d
}
tf.protocol = ATA_PROT_PIO;
-
- /* presence detection using polling IDENTIFY? */
- if (flags & ATA_READID_DETECT)
- tf.flags |= ATA_TFLAG_POLLING;
+ tf.flags |= ATA_TFLAG_POLLING; /* for polling presence detection */
err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE,
id, sizeof(id[0]) * ATA_ID_WORDS);
if (err_mask) {
- if ((flags & ATA_READID_DETECT) &&
- (err_mask & AC_ERR_NODEV_HINT)) {
+ if (err_mask & AC_ERR_NODEV_HINT) {
DPRINTK("ata%u.%d: NODEV after polling detection\n",
ap->id, dev->devno);
return -ENOENT;
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 5aa7f09..d84bb65 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1690,9 +1690,6 @@ static int ata_eh_revalidate_and_attach(
ata_class_enabled(ehc->classes[dev->devno])) {
dev->class = ehc->classes[dev->devno];
- if (ap->flags & ATA_FLAG_DETECT_POLLING)
- readid_flags |= ATA_READID_DETECT;
-
rc = ata_dev_read_id(dev, &dev->class, readid_flags,
dev->id);
if (rc == 0) {
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 2d532da..a367317 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -42,8 +42,6 @@ struct ata_scsi_args {
enum {
/* flags for ata_dev_read_id() */
ATA_READID_POSTRESET = (1 << 0), /* reading ID after reset */
- ATA_READID_DETECT = (1 << 1), /* perform presence detection
- * using polling IDENTIFY */
};
extern struct workqueue_struct *ata_aux_wq;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 9080789..26abd6a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -176,8 +176,6 @@ enum {
ATA_FLAG_SKIP_D2H_BSY = (1 << 12), /* can't wait for the first D2H
* Register FIS clearing BSY */
ATA_FLAG_DEBUGMSG = (1 << 13),
- ATA_FLAG_DETECT_POLLING = (1 << 14), /* detect device presence by
- * polling IDENTIFY */
/* The following flag belongs to ap->pflags but is kept in
* ap->flags because it's referenced in many LLDs and will be
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] libata: always use polling IDENTIFY
2006-12-03 10:30 [PATCH] libata: always use polling IDENTIFY Tejun Heo
@ 2006-12-03 11:14 ` Jeff Garzik
2006-12-03 12:34 ` [PATCH RESEND] " Tejun Heo
2006-12-03 12:56 ` [PATCH] " Alan
2 siblings, 0 replies; 6+ messages in thread
From: Jeff Garzik @ 2006-12-03 11:14 UTC (permalink / raw)
To: Tejun Heo; +Cc: Alan Cox, linux-ide
Tejun Heo wrote:
> libata switched to IRQ-driven IDENTIFY when IRQ-driven PIO was
> introduced. This has caused a lot of problems including device
> misdetection and phantom device.
>
> ATA_FLAG_DETECT_POLLING was added recently to selectively use polling
> IDENTIFY on problemetic drivers but many controllers and devices are
> affected by this problem and trying to adding ATA_FLAG_DETECT_POLLING
> for each such case is diffcult and not very rewarding.
>
> This patch makes libata always use polling IDENTIFY. This is
> consistent with libata's original behavior and drivers/ide's behavior.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
ACK, but does not apply to #upstream[1], which includes the tj-upstream
that I pulled.
Jeff
[1] 8e16f941226f15622fbbc416a1f3d8705001a191
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH RESEND] libata: always use polling IDENTIFY
2006-12-03 10:30 [PATCH] libata: always use polling IDENTIFY Tejun Heo
2006-12-03 11:14 ` Jeff Garzik
@ 2006-12-03 12:34 ` Tejun Heo
2006-12-03 12:58 ` Jeff Garzik
2006-12-03 12:56 ` [PATCH] " Alan
2 siblings, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2006-12-03 12:34 UTC (permalink / raw)
To: Jeff Garzik, Alan Cox, linux-ide
libata switched to IRQ-driven IDENTIFY when IRQ-driven PIO was
introduced. This has caused a lot of problems including device
misdetection and phantom device.
ATA_FLAG_DETECT_POLLING was added recently to selectively use polling
IDENTIFY on problemetic drivers but many controllers and devices are
affected by this problem and trying to adding ATA_FLAG_DETECT_POLLING
for each such case is diffcult and not very rewarding.
This patch makes libata always use polling IDENTIFY. This is
consistent with libata's original behavior and drivers/ide's behavior.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Regenerated against the current #upstream.
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 788a269..a2d84f7 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -105,9 +105,8 @@ enum {
PIIX_FLAG_AHCI = (1 << 27), /* AHCI possible */
PIIX_FLAG_CHECKINTR = (1 << 28), /* make sure PCI INTx enabled */
- PIIX_PATA_FLAGS = ATA_FLAG_SLAVE_POSS | ATA_FLAG_DETECT_POLLING,
- PIIX_SATA_FLAGS = ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR |
- ATA_FLAG_DETECT_POLLING,
+ PIIX_PATA_FLAGS = ATA_FLAG_SLAVE_POSS,
+ PIIX_SATA_FLAGS = ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR,
/* combined mode. if set, PATA is channel 0.
* if clear, PATA is channel 1.
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 8832763..f8ec389 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1473,16 +1473,12 @@ int ata_dev_read_id(struct ata_device *d
}
tf.protocol = ATA_PROT_PIO;
-
- /* presence detection using polling IDENTIFY? */
- if (flags & ATA_READID_DETECT)
- tf.flags |= ATA_TFLAG_POLLING;
+ tf.flags |= ATA_TFLAG_POLLING; /* for polling presence detection */
err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE,
id, sizeof(id[0]) * ATA_ID_WORDS);
if (err_mask) {
- if ((flags & ATA_READID_DETECT) &&
- (err_mask & AC_ERR_NODEV_HINT)) {
+ if (err_mask & AC_ERR_NODEV_HINT) {
DPRINTK("ata%u.%d: NODEV after polling detection\n",
ap->id, dev->devno);
return -ENOENT;
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 2aad7b7..76a85df 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1692,9 +1692,6 @@ static int ata_eh_revalidate_and_attach(
ata_class_enabled(ehc->classes[dev->devno])) {
dev->class = ehc->classes[dev->devno];
- if (ap->flags & ATA_FLAG_DETECT_POLLING)
- readid_flags |= ATA_READID_DETECT;
-
rc = ata_dev_read_id(dev, &dev->class, readid_flags,
dev->id);
if (rc == 0) {
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 1ff3f59..107b2b5 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -42,8 +42,6 @@ struct ata_scsi_args {
enum {
/* flags for ata_dev_read_id() */
ATA_READID_POSTRESET = (1 << 0), /* reading ID after reset */
- ATA_READID_DETECT = (1 << 1), /* perform presence detection
- * using polling IDENTIFY */
};
extern struct workqueue_struct *ata_aux_wq;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 8b57b6a..202283b 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -176,9 +176,7 @@ enum {
ATA_FLAG_SKIP_D2H_BSY = (1 << 12), /* can't wait for the first D2H
* Register FIS clearing BSY */
ATA_FLAG_DEBUGMSG = (1 << 13),
- ATA_FLAG_DETECT_POLLING = (1 << 14), /* detect device presence by
- * polling IDENTIFY */
- ATA_FLAG_SETXFER_POLLING= (1 << 15), /* use polling for SETXFER */
+ ATA_FLAG_SETXFER_POLLING= (1 << 14), /* use polling for SETXFER */
/* The following flag belongs to ap->pflags but is kept in
* ap->flags because it's referenced in many LLDs and will be
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] libata: always use polling IDENTIFY
2006-12-03 10:30 [PATCH] libata: always use polling IDENTIFY Tejun Heo
2006-12-03 11:14 ` Jeff Garzik
2006-12-03 12:34 ` [PATCH RESEND] " Tejun Heo
@ 2006-12-03 12:56 ` Alan
2006-12-03 13:02 ` Jeff Garzik
2 siblings, 1 reply; 6+ messages in thread
From: Alan @ 2006-12-03 12:56 UTC (permalink / raw)
To: Tejun Heo; +Cc: Jeff Garzik, linux-ide
On Sun, 3 Dec 2006 19:30:32 +0900
Tejun Heo <htejun@gmail.com> wrote:
> libata switched to IRQ-driven IDENTIFY when IRQ-driven PIO was
> introduced. This has caused a lot of problems including device
> misdetection and phantom device.
>
> ATA_FLAG_DETECT_POLLING was added recently to selectively use polling
> IDENTIFY on problemetic drivers but many controllers and devices are
> affected by this problem and trying to adding ATA_FLAG_DETECT_POLLING
> for each such case is diffcult and not very rewarding.
>
> This patch makes libata always use polling IDENTIFY. This is
> consistent with libata's original behavior and drivers/ide's behavior.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
Acked-by: Alan Cox <alan@redhat.com>
One possibility however would be to invert the flag we have now so we can
try turning -on- IRQ polling on some devices in future and maybe figure
it out that way around ?
Alan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] libata: always use polling IDENTIFY
2006-12-03 12:34 ` [PATCH RESEND] " Tejun Heo
@ 2006-12-03 12:58 ` Jeff Garzik
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Garzik @ 2006-12-03 12:58 UTC (permalink / raw)
To: Tejun Heo; +Cc: Alan Cox, linux-ide
Tejun Heo wrote:
> libata switched to IRQ-driven IDENTIFY when IRQ-driven PIO was
> introduced. This has caused a lot of problems including device
> misdetection and phantom device.
>
> ATA_FLAG_DETECT_POLLING was added recently to selectively use polling
> IDENTIFY on problemetic drivers but many controllers and devices are
> affected by this problem and trying to adding ATA_FLAG_DETECT_POLLING
> for each such case is diffcult and not very rewarding.
>
> This patch makes libata always use polling IDENTIFY. This is
> consistent with libata's original behavior and drivers/ide's behavior.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
applied
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libata: always use polling IDENTIFY
2006-12-03 12:56 ` [PATCH] " Alan
@ 2006-12-03 13:02 ` Jeff Garzik
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Garzik @ 2006-12-03 13:02 UTC (permalink / raw)
To: Alan; +Cc: Tejun Heo, linux-ide
Alan wrote:
> On Sun, 3 Dec 2006 19:30:32 +0900
> Tejun Heo <htejun@gmail.com> wrote:
>
>> libata switched to IRQ-driven IDENTIFY when IRQ-driven PIO was
>> introduced. This has caused a lot of problems including device
>> misdetection and phantom device.
>>
>> ATA_FLAG_DETECT_POLLING was added recently to selectively use polling
>> IDENTIFY on problemetic drivers but many controllers and devices are
>> affected by this problem and trying to adding ATA_FLAG_DETECT_POLLING
>> for each such case is diffcult and not very rewarding.
>>
>> This patch makes libata always use polling IDENTIFY. This is
>> consistent with libata's original behavior and drivers/ide's behavior.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>
> Acked-by: Alan Cox <alan@redhat.com>
>
> One possibility however would be to invert the flag we have now so we can
> try turning -on- IRQ polling on some devices in future and maybe figure
> it out that way around ?
Agreed, though it is low priority.
You'll note that Tejun's change only applies to the [large] set of
drivers using libata-sff, rather than all drivers. Modern, unbridged
SATA controllers (ahci, sata_sil24) continue to do their own interrupts
based on received packets (FIS's) through the use of the higher level
libata hooks.
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-12-03 13:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-12-03 10:30 [PATCH] libata: always use polling IDENTIFY Tejun Heo
2006-12-03 11:14 ` Jeff Garzik
2006-12-03 12:34 ` [PATCH RESEND] " Tejun Heo
2006-12-03 12:58 ` Jeff Garzik
2006-12-03 12:56 ` [PATCH] " Alan
2006-12-03 13:02 ` Jeff Garzik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).