[PATCH] libata: implement and use ata_wait_idle_quiet()

[PATCH] libata: implement and use ata_wait_idle_quiet()

[Tejun Heo]

ata_wait_idle() complains when the device isn't idle 10ms wait.
However, this function sometimes is used on empty ports.  As SATA
controllers report arbitrary status on empty ports, this causes
spurious warning messages.

* separate ata_wait_idle_quiet() from ata_wait_idle()
* use _quiet version in ata_irq_on()
* fix warning message in ata_wait_idle() to use ata_port_printk()
  while at it

This makes spurious abnormal status messages seen in many sff
controllers.

Signed-off-by: Tejun Heo <htejun@gmail.com>

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 2ffcca0..22cd6e1 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -57,7 +57,8 @@ u8 ata_irq_on(struct ata_port *ap)
 	ap->last_ctl = ap->ctl;
 
 	iowrite8(ap->ctl, ioaddr->ctl_addr);
-	tmp = ata_wait_idle(ap);
+	/* we might be called on empty port for thawing, wait quietly */
+	tmp = ata_wait_idle_quiet(ap);
 
 	ap->ops->irq_clear(ap);
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 0a87ba8..6de878d 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1095,9 +1095,8 @@ static inline u8 ata_busy_wait(struct ata_port *ap, unsigned int bits,
 	return status;
 }
 
-
 /**
- *	ata_wait_idle - Wait for a port to be idle.
+ *	ata_wait_idle_quiet - Wait for a port to be idle.
  *	@ap: Port to wait for.
  *
  *	Waits up to 10ms for port's BUSY and DRQ signals to clear.
@@ -1106,15 +1105,30 @@ static inline u8 ata_busy_wait(struct ata_port *ap, unsigned int bits,
  *	LOCKING:
  *	Inherited from caller.
  */
+static inline u8 ata_wait_idle_quiet(struct ata_port *ap)
+{
+	return ata_busy_wait(ap, ATA_BUSY | ATA_DRQ, 1000);
+}
 
+/**
+ *	ata_wait_idle - Wait for a port to be idle, whine on failure.
+ *	@ap: Port to wait for.
+ *
+ *	Waits up to 10ms for port's BUSY and DRQ signals to clear.
+ *	Returns final value of status register.  This function whines
+ *	if port doesn't become idle in 10ms.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
 static inline u8 ata_wait_idle(struct ata_port *ap)
 {
-	u8 status = ata_busy_wait(ap, ATA_BUSY | ATA_DRQ, 1000);
+	u8 status = ata_wait_idle_quiet(ap);
 
 	if (status != 0xff && (status & (ATA_BUSY | ATA_DRQ))) {
 		if (ata_msg_warn(ap))
-			printk(KERN_WARNING "ATA: abnormal status 0x%X on port 0x%p\n",
-				status, ap->ioaddr.status_addr);
+			ata_port_printk(ap, KERN_WARNING,
+					"abnormal status 0x%x\n", status);
 	}
 
 	return status;

Re: [PATCH] libata: implement and use ata_wait_idle_quiet()

[Jeff Garzik]

Tejun Heo wrote:
> ata_wait_idle() complains when the device isn't idle 10ms wait.
> However, this function sometimes is used on empty ports.  As SATA
> controllers report arbitrary status on empty ports, this causes
> spurious warning messages.
> 
> * separate ata_wait_idle_quiet() from ata_wait_idle()
> * use _quiet version in ata_irq_on()
> * fix warning message in ata_wait_idle() to use ata_port_printk()
>   while at it
> 
> This makes spurious abnormal status messages seen in many sff
> controllers.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

My feeling is that this will also hide bugs, such as in the AHCI case

	Jeff

Re: [PATCH] libata: implement and use ata_wait_idle_quiet()

[Alan Cox]

> > This makes spurious abnormal status messages seen in many sff
> > controllers.
> > 
> > Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> My feeling is that this will also hide bugs, such as in the AHCI case

I think its the right thing to do in the end at least for the SFF cases
(can't comment on AHCI). Perhaps we should make that version printk to a
different printk level for now or if ATA_DEBUG is enabled ?