scsi: Update Aic94xx SAS/SATA Linux open source device

PATCH NAK Gilbert Wu 3 participants 5 messages Aug 31, 2007 SCSI

lore

Share & tools

lore

Gilbert Wu

author

2007-08-31 18:10 UTC #0 lore raw

PATCH 1/1 scsi: Update Aic94xx SAS/SATA Linux open source device

Subject: [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device driver to fix smartctl utility problem.

Contribution:
Gilbert Wu <[email protected]>

Change Log:

1. Fixed the problem that "smartctl -a /dev/some_sata_disk -d ata" does not work on aic94xx device drive with SATA devices. The smartctl utility send down ATA smart command to aic94xx driver and fail to get the result which is stored on ATA output register.
In normal case, Aic94xx’s firmware does not return the value of ATA output register to device driver due to performance reason. To solve this issue then the driver need to turn on internal flag (CSMI_TASK) for particular ATA command to enable firmware to return the value of ATA output register.

Patch: apply to aic94xx-sas-2.6.git development tree

Signed-off-by Gilbert Wu <[email protected]>

diff -urN old/drivers/scsi/aic94xx/aic94xx_task.c
new/drivers/scsi/aic94xx/aic94xx_task.c

drivers/scsi/aic94xx/aic94xx_task.c +15 -0

@@ -33,6 +33,11 @@
 static void asd_unbuild_smp_ascb(struct asd_ascb *a);
 static void asd_unbuild_ssp_ascb(struct asd_ascb *a);
 
+#define ATA_SMART_COMMAND       0xb0
+#define ATA_DEVICE_DIAGNOSTIC   0x90
+#define ATA_CHECK_POWER_MODE    0xe5
+#define ATA_SMART_STATUS        0xda
+
 static inline void asd_can_dequeue(struct asd_ha_struct *asd_ha, int num)
 {
 	unsigned long flags;

@@ -253,6 +258,7 @@
 		break;
 	case TC_SSP_RESP:
 	case TC_ATA_RESP:
+	case TC_CSMI:
 		ts->resp = SAS_TASK_COMPLETE;
 		ts->stat = SAS_PROTO_RESPONSE;
 		asd_get_response_tasklet(ascb, dl);

@@ -427,6 +433,15 @@
 			flags |= STP_AFFIL_POLICY;
 		scb->ata_task.flags = flags;
 	}
+
+	if ((scb->ata_task.fis.command == ATA_SMART_COMMAND &&
+		scb->ata_task.fis.features == ATA_SMART_STATUS)
+		|| scb->ata_task.fis.command == ATA_DEVICE_DIAGNOSTIC
+		|| scb->ata_task.fis.command == ATA_CHECK_POWER_MODE)
+	{
+		scb->ata_task.ata_flags|=CSMI_TASK;
+	}
+
 	ascb->tasklet_complete = asd_task_tasklet_complete;
 
 	if (likely(!task->ata_task.device_control_reg_update))

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Jeff Garzik

NAK

2007-08-31 18:27 UTC #1 lore raw

Quoted 23 lines · Gilbert Wu

Subject: [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device driver to fix smartctl utility problem.
Contribution:
Gilbert Wu <[email protected]>
Change Log:
1. Fixed the problem that "smartctl -a /dev/some_sata_disk -d ata" does not work on aic94xx device drive with SATA devices. The smartctl utility send down ATA smart command to aic94xx driver and fail to get the result which is stored on ATA output register.
In normal case, Aic94xx’s firmware does not return the value of ATA output register to device driver due to performance reason. To solve this issue then the driver need to turn on internal flag (CSMI_TASK) for particular ATA command to enable firmware to return the value of ATA output register.

Quoted 3 lines

Patch: apply to aic94xx-sas-2.6.git development tree
Signed-off-by Gilbert Wu <[email protected]>

NAK

1) re-invents ATA command constants (a third time!).

Fix: use the constants in include/linux/ata.h

2) ignores the large number of ATA commands that also need register output.

Fix: invert the test. the only time when we do NOT want the ATA register output is upon successful completion of "hot path" READ/WRITE commands. For ALL other cases, we need the D2H FIS and/or SDB FIS data reflecting register state.

3) Patches are normally imported by automated tools, and your email body is copied directly into the permanent kernel changelog, archived for all eternity. As such, you should format your patch description in a similar fashion to other patches.

Fixes:
a) Subject line way too long and redundant. Use:

[PATCH] aic94xx: fix smartctl utility problem

Execute 'git log' in the kernel repository for more examples. The email subject is always copied into the kernel changelog as the FIRST LINE of the change. This is a one-line summary of your patch.

b) Do not write "Subject: [...]" in email body. Make this the subject of your email, per guidelines at http://linux.yyz.us/patch-format.html and Documentation/SubmittingPatches

c) Remove the redundant line
Contribution:
Gilbert Wu <[email protected]>
(this is redundant to your email address in the email, and the Signed-off-by line)

d) Remove the redundant line
Change Log:
(we already know its a changelog)

e) Move temporal and meta-comments after the "---" separator, so that this text is not copied into the permanent kernel changelog. See Documentation/SubmittingPatches for more info.

Regards,

Jeff

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gilbert Wu

author

2007-08-31 21:07 UTC #2 lore raw

Quoted 71 lines · Jeff Garzik

Quoted 23 lines · Gilbert Wu
Subject: [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device driver to fix smartctl utility problem.
Contribution:
Gilbert Wu <[email protected]>
Change Log:
1. Fixed the problem that "smartctl -a /dev/some_sata_disk -d ata" does not work on aic94xx device drive with SATA devices. The smartctl utility send down ATA smart command to aic94xx driver and fail to get the result which is stored on ATA output register.
In normal case, Aic94xx’s firmware does not return the value of ATA output register to device driver due to performance reason. To solve this issue then the driver need to turn on internal flag (CSMI_TASK) for particular ATA command to enable firmware to return the value of ATA output register.
Quoted 3 lines
Patch: apply to aic94xx-sas-2.6.git development tree
Signed-off-by Gilbert Wu <[email protected]>
NAK
1) re-invents ATA command constants (a third time!).
Fix: use the constants in include/linux/ata.h
2) ignores the large number of ATA commands that also need register output.
Fix: invert the test. the only time when we do NOT want the ATA register output is upon successful completion of "hot path" READ/WRITE commands. For ALL other cases, we need the D2H FIS and/or SDB FIS data reflecting register state.
3) Patches are normally imported by automated tools, and your email body is copied directly into the permanent kernel changelog, archived for all eternity. As such, you should format your patch description in a similar fashion to other patches.
Fixes:
a) Subject line way too long and redundant. Use:
[PATCH] aic94xx: fix smartctl utility problem
Execute 'git log' in the kernel repository for more examples. The email subject is always copied into the kernel changelog as the FIRST LINE of the change. This is a one-line summary of your patch.
b) Do not write "Subject: [...]" in email body. Make this the subject of your email, per guidelines at http://linux.yyz.us/patch-format.html
and Documentation/SubmittingPatches
c) Remove the redundant line
Contribution:
Gilbert Wu <[email protected]>
(this is redundant to your email address in the email, and the Signed-off-by line)
d) Remove the redundant line
Change Log:
(we already know its a changelog)

Quoted 8 lines

e) Move temporal and meta-comments after the "---" separator, so that this text is not copied into the permanent kernel changelog. See Documentation/SubmittingPatches for more info.
Regards,
Jeff

Hi Jeff,

Thanks for your comments. I go through each item and rewrite the patch.

Thanks!
Gilbert
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Douglas Gilbert

2007-08-31 21:32 UTC #3 lore raw

In reply to Gilbert Wu

Quoted 9 lines · Gilbert Wu

Subject: [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device driver to fix smartctl utility problem.
Contribution:
Gilbert Wu <[email protected]>
Change Log:

Quoted 10 lines

1. Fixed the problem that "smartctl -a /dev/some_sata_disk -d ata" does not work on aic94xx device drive with SATA devices. The smartctl utility send down ATA smart command to aic94xx driver and fail to get the result which is stored on ATA output register.
In normal case, Aic94xx’s firmware does not return the value of ATA output register to device driver due to performance reason. To solve this issue then the driver need to turn on internal flag (CSMI_TASK) for particular ATA command to enable firmware to return the value of ATA output register.

Recent versions of smartmontools (say 5.38 from CVS at sourceforge) don't need the '-d ata' option any more.

If smartctl or smartd see /dev/sd<x> in Linux, they will assume a SCSI disk, do an INQUIRY and if they see
"ATA " in the T10 vendor identification field then an
ATA (most likely SATA) disk is assumed behind a SAT layer. Thereafter ATA commands will be issued, each wrapped in a SCSI ATA PASS-THROUGH(16) command.

Perhaps you might check that this works with the aic94xx driver. [It does work with MPT SAS HBAs.]

Doug Gilbert
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gilbert Wu

author

2007-08-31 23:43 UTC #4 lore raw

Quoted 31 lines · Douglas Gilbert

Quoted 9 lines · Gilbert Wu
Subject: [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device driver to fix smartctl utility problem.
Contribution:
Gilbert Wu <[email protected]>
Change Log:
Quoted 10 lines
1. Fixed the problem that "smartctl -a /dev/some_sata_disk -d ata" does not work on aic94xx device drive with SATA devices. The smartctl utility send down ATA smart command to aic94xx driver and fail to get the result which is stored on ATA output register.
In normal case, Aic94xx’s firmware does not return the value of ATA output register to device driver due to performance reason. To solve this issue then the driver need to turn on internal flag (CSMI_TASK) for particular ATA command to enable firmware to return the value of ATA output register.
Recent versions of smartmontools (say 5.38 from CVS at sourceforge) don't need the '-d ata' option any more.
If smartctl or smartd see /dev/sd<x> in Linux, they will assume a SCSI disk, do an INQUIRY and if they see
"ATA " in the T10 vendor identification field then an
ATA (most likely SATA) disk is assumed behind a SAT layer. Thereafter ATA commands will be issued, each wrapped in a SCSI ATA PASS-THROUGH(16) command.

Quoted 4 lines

Perhaps you might check that this works with the aic94xx driver. [It does work with MPT SAS HBAs.]
Doug Gilbert

The smartctl-5.38 works fine with aic94xx after fixed the ATA register output problem. I also verify it by adding debug message that smartctl-5.8 utility sends ATA smart command to aic94xx driver with SATA device.

Thanks!
Gilbert
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html