[PATCH 1/1] libata: dump full ATA firmware revision to dmesg

[PATCH 1/1] libata: dump full ATA firmware revision to dmesg

[Eric D. Mudama]

SCSI only supports a 4-character firmware revision string, while ATA
uses 8 characters.  Since it can be useful for debugging drive/host
issues, and because not everyone has a working hdparm on their system,
dump the full ATA firmware revision into dmesg at drive detection.
Only affects ATA devices connected via libata, and not ATAPI (since
they likely use SCSI conventions anyway).

Sample output from my dmesg:

ata1.00: ATA-7, max UDMA/133, 488395055 sectors: LBA48 NCQ (depth 0/32)
ata1.00: ata1: dev 0 multi count 16
ata1.00: ata1: Firmware Revision: 3.AAC
ata1.00: configured for UDMA/133
ata2.00: ATA-7, max UDMA/133, 195813072 sectors: LBA48 NCQ (depth 0/32)
ata2.00: ata2: dev 0 multi count 16
ata2.00: ata2: Firmware Revision: VA111910
ata2.00: configured for UDMA/133
scsi 0:0:0:0: Direct-Access     ATA      ST3250820AS      3.AA PQ: 0 ANSI: 5
scsi 1:0:0:0: Direct-Access     ATA      Maxtor 6V100E0   VA11 PQ: 0 ANSI: 5

Signed-off-by: Eric D. Mudama <edmudama@gmail.com>

---

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a388a8d..7dc4dad 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1607,6 +1607,7 @@ int ata_dev_configure(struct ata_device *dev)
        const u16 *id = dev->id;
        unsigned int xfer_mask;
        char revbuf[7];         /* XYZ-99\0 */
+       char fwrevbuf[9];
        int rc;

        if (!ata_dev_enabled(dev) && ata_msg_info(ap)) {
@@ -1721,6 +1722,16 @@ int ata_dev_configure(struct ata_device *dev)
                                        ap->id, dev->devno, dev->multi_count);
                }

+               if (ata_msg_drv(ap) && print_info) {
+                       /* SCSI only uses 4-char revisions, dump full
8 chars from ATA */
+                       ata_id_c_string(dev->id, fwrevbuf, ATA_ID_FW_REV_OFS,
+                                       sizeof(fwrevbuf));
+
+                       ata_dev_printk(dev, KERN_INFO,
+                                      "ata%u: Firmware Revision: %s\n",
+                                      ap->id, fwrevbuf);
+               }
+
                dev->cdb_len = 16;
        }

[PATCH 1/1] libata: dump full ATA firmware revision to dmesg

[Eric D. Mudama]

SCSI only supports a 4-character firmware revision string, while ATA
uses 8 characters.  Record this firmware revision string into dmesg.

Signed-off-by: Eric D. Mudama <edmudama@gmail.com>

---

Patch is against 2.6.20-rc6 pull via GIT.  Resubmitting to (hopefully)
fix linewrap and bad formatting of the commit message.

This is useful for drive vendors debugging issues with ATA devices and
libata, since not everyone can easily run hdparm on their systems.

Sample output from my dmesg, showing the truncated firmware revisions:

ata1.00: ATA-7, max UDMA/133, 488395055 sectors: LBA48 NCQ (depth 0/32)
ata1.00: ata1: dev 0 multi count 16
ata1.00: ata1: Firmware Revision: 3.AAC
ata1.00: configured for UDMA/133
ata2.00: ATA-7, max UDMA/133, 195813072 sectors: LBA48 NCQ (depth 0/32)
ata2.00: ata2: dev 0 multi count 16
ata2.00: ata2: Firmware Revision: VA111910
ata2.00: configured for UDMA/133
scsi 0:0:0:0: Direct-Access     ATA      ST3250820AS      3.AA PQ: 0 ANSI: 5
scsi 1:0:0:0: Direct-Access     ATA      Maxtor 6V100E0   VA11 PQ: 0 ANSI: 5


diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a388a8d..7dc4dad 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1607,6 +1607,7 @@ int ata_dev_configure(struct ata_device *dev)
       const u16 *id = dev->id;
       unsigned int xfer_mask;
       char revbuf[7];         /* XYZ-99\0 */
+       char fwrevbuf[9];
       int rc;

       if (!ata_dev_enabled(dev) && ata_msg_info(ap)) {
@@ -1721,6 +1722,16 @@ int ata_dev_configure(struct ata_device *dev)
                                       ap->id, dev->devno, dev->multi_count);
               }

+               if (ata_msg_drv(ap) && print_info) {
+                       /* SCSI only uses 4-char revisions, dump full
8 chars from ATA */
+                       ata_id_c_string(dev->id, fwrevbuf, ATA_ID_FW_REV_OFS,
+                                       sizeof(fwrevbuf));
+
+                       ata_dev_printk(dev, KERN_INFO,
+                                      "ata%u: Firmware Revision: %s\n",
+                                      ap->id, fwrevbuf);
+               }
+
               dev->cdb_len = 16;
       }

Re: [PATCH 1/1] libata: dump full ATA firmware revision to dmesg

[Jeff Garzik]

Eric D. Mudama wrote:
> SCSI only supports a 4-character firmware revision string, while ATA
> uses 8 characters.  Since it can be useful for debugging drive/host
> issues, and because not everyone has a working hdparm on their system,
> dump the full ATA firmware revision into dmesg at drive detection.
> Only affects ATA devices connected via libata, and not ATAPI (since
> they likely use SCSI conventions anyway).
> 
> Sample output from my dmesg:
> 
> ata1.00: ATA-7, max UDMA/133, 488395055 sectors: LBA48 NCQ (depth 0/32)
> ata1.00: ata1: dev 0 multi count 16
> ata1.00: ata1: Firmware Revision: 3.AAC
> ata1.00: configured for UDMA/133
> ata2.00: ATA-7, max UDMA/133, 195813072 sectors: LBA48 NCQ (depth 0/32)
> ata2.00: ata2: dev 0 multi count 16
> ata2.00: ata2: Firmware Revision: VA111910
> ata2.00: configured for UDMA/133
> scsi 0:0:0:0: Direct-Access     ATA      ST3250820AS      3.AA PQ: 0 
> ANSI: 5
> scsi 1:0:0:0: Direct-Access     ATA      Maxtor 6V100E0   VA11 PQ: 0 
> ANSI: 5
> 
> Signed-off-by: Eric D. Mudama <edmudama@gmail.com>

hmmmm.  While I agree that exposing the full firmware rev is a good 
thing to do, I'm a bit embarrassed as it is about libata's dmesg spam, 
and I try to resist increasing it.  As such, I think that firmware rev 
on a line all by itself is a bit "greedy" by those standards.

Additionally -- and this is not your fault since the previous line is 
the same -- the "ata2.00: ata2: [dev 0]" is redundant.  The "ata2.00" is 
all we really need to know which bus and device we're referring to.

I would rather revamp things to look something like

	...
	ata2.00: ATA-7, Maxtor 6V100E0, VA111910, max UDMA/133
	ata2.00: 195813072 sectors, multi 16, LBA48 NCQ (depth 0/32)
	...

I realize that's more work than just a simple addition, alas...

Regards,

	Jeff