[PATCHSET 3/9] misc ata_bus_probe() updates

[PATCHSET 3/9] misc ata_bus_probe() updates

[Tejun Heo]

Hello, again.

This patchset contains three patches updating ata_bus_probe() and
related parts.  These patches don't make any functional changes.  Only
printk messages and code structure are changed.  This is in
preparation for new EH.

This patchset is against.

  upstream (c2a6585296009379e0f4eff39cdcb108b457ebf2)
  + [1] misc-reset-updates patchset (repost)
  + [2] implement-and-use-ata_wait_register patchset (repost)

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.ide/9495
[2] http://article.gmane.org/gmane.linux.ide/9499

[PATCH 1/3] libata: disable failed devices only once in ata_bus_probe()

[Tejun Heo]

Devices which consumed all their changes used to be disabled every
iteration.  This causes unnecessary noise in the console output.
Disable once and leave alone.

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

---

 drivers/scsi/libata-core.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

54e2d4ab9771cc5d7c228ee0e2087f009df0efb4
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 56e1cc6..e42a8f3 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1411,12 +1411,9 @@ static int ata_bus_probe(struct ata_port
 	/* read IDENTIFY page and configure devices */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		dev = &ap->device[i];
-		dev->class = classes[i];
 
-		if (!tries[i]) {
-			ata_down_xfermask_limit(ap, dev, 1);
-			ata_dev_disable(ap, dev);
-		}
+		if (tries[i])
+			dev->class = classes[i];
 
 		if (!ata_dev_enabled(dev))
 			continue;
@@ -1477,6 +1474,11 @@ static int ata_bus_probe(struct ata_port
 			tries[dev->devno] = 0;
 	}
 
+	if (!tries[dev->devno]) {
+		ata_down_xfermask_limit(ap, dev, 1);
+		ata_dev_disable(ap, dev);
+	}
+
 	goto retry;
 }
 
-- 
1.2.4

Re: [PATCH 1/3] libata: disable failed devices only once in ata_bus_probe()

[Jeff Garzik]

Tejun Heo wrote:
> Devices which consumed all their changes used to be disabled every
> iteration.  This causes unnecessary noise in the console output.
> Disable once and leave alone.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

applied 1-2 out of 3

[PATCH 3/3] libata: prefix sata register print outs with 0x

[Tejun Heo]

Prefix SStatus and SControl values with 0x as they are hex.

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

---

 drivers/scsi/libata-core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

f95a0f4882a17839102844fa0afe74a88e554800
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index f1abe75..808a978 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1520,11 +1520,11 @@ static void sata_print_link_status(struc
 	if (sata_dev_present(ap)) {
 		tmp = (sstatus >> 4) & 0xf;
 		printk(KERN_INFO
-		       "ata%u: SATA link up %s (SStatus %X SControl %X)\n",
+		       "ata%u: SATA link up %s (SStatus 0x%X SControl 0x%X)\n",
 		       ap->id, sata_spd_string(tmp), sstatus, scontrol);
 	} else {
 		printk(KERN_INFO
-		       "ata%u: SATA link down (SStatus %X SControl %X)\n",
+		       "ata%u: SATA link down (SStatus 0x%X SControl 0x%X)\n",
 		       ap->id, sstatus, scontrol);
 	}
 }
-- 
1.2.4

Re: [PATCH 3/3] libata: prefix sata register print outs with 0x

[Jeff Garzik]

Tejun Heo wrote:
> Prefix SStatus and SControl values with 0x as they are hex.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

NAK, libata always prints out device id, register, etc. values in hex. 
Basically anything other than things like sectors or bytes is printed in 
hex.  Therefore, the "0x" is largely redundant.

The prefix/base is implicit in the context.

	Jeff

Re: [PATCH 3/3] libata: prefix sata register print outs with 0x

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Prefix SStatus and SControl values with 0x as they are hex.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> NAK, libata always prints out device id, register, etc. values in hex. 
> Basically anything other than things like sectors or bytes is printed in 
> hex.  Therefore, the "0x" is largely redundant.
> 
> The prefix/base is implicit in the context.
> 

The reason why I submitted this patch was messages like the following...

ata2: SATA link down (SStatus 11 SControl 310)

When it was three digits, it was okay.  But two or just one digits look 
just weird.  Maybe what's needed is %03.

Well, no big deal either way.

-- 
tejun

[PATCH 2/3] libata: cosmetic update to ata_bus_probe()

[Tejun Heo]

Move ata_set_mode() failure handling outside of ap->ops->set_mode if
clause such that it can handle ap->ops->set_mode failures after it's
updated.

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

---

 drivers/scsi/libata-core.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

8ed14e7229110cf9e338ba866fdca78373d1215d
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index e42a8f3..f1abe75 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1441,12 +1441,12 @@ static int ata_bus_probe(struct ata_port
 				break;
 			}
 		rc = 0;
-	} else {
+	} else
 		rc = ata_set_mode(ap, &dev);
-		if (rc) {
-			down_xfermask = 1;
-			goto fail;
-		}
+
+	if (rc) {
+		down_xfermask = 1;
+		goto fail;
 	}
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++)
-- 
1.2.4