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

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

[Gilbert Wu]

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

Contribution:
   Gilbert Wu <gilbert_wu@adaptec.com>

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 <gilbert_wu@adaptec.com>




diff -urN old/drivers/scsi/aic94xx/aic94xx_task.c
new/drivers/scsi/aic94xx/aic94xx_task.c
--- old/drivers/scsi/aic94xx/aic94xx_task.c	2007-08-30 16:32:12.000000000 -0700
+++ new/drivers/scsi/aic94xx/aic94xx_task.c	2007-08-30 16:31:50.000000000 -0700
@@ -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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device

[Jeff Garzik]

Gilbert Wu wrote:
> Subject:  [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device
> driver to fix smartctl utility problem.
> 
> Contribution:
>    Gilbert Wu <gilbert_wu@adaptec.com>
> 
> 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 <gilbert_wu@adaptec.com>

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 <gilbert_wu@adaptec.com>
(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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device

[Gilbert Wu]

On Fri, 2007-08-31 at 14:27 -0400, Jeff Garzik wrote:
> Gilbert Wu wrote:
> > Subject:  [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device
> > driver to fix smartctl utility problem.
> > 
> > Contribution:
> >    Gilbert Wu <gilbert_wu@adaptec.com>
> > 
> > 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 <gilbert_wu@adaptec.com>
> 
> 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 <gilbert_wu@adaptec.com>
> (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
> 

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device

[Douglas Gilbert]

Gilbert Wu wrote:
> Subject:  [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device
> driver to fix smartctl utility problem.
> 
> Contribution:
>    Gilbert Wu <gilbert_wu@adaptec.com>
> 
> 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.

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device

[Gilbert Wu]

On Fri, 2007-08-31 at 17:31 -0400, Douglas Gilbert wrote:
> Gilbert Wu wrote:
> > Subject:  [PATCH] scsi: Update Aic94xx SAS/SATA Linux open source device
> > driver to fix smartctl utility problem.
> > 
> > Contribution:
> >    Gilbert Wu <gilbert_wu@adaptec.com>
> > 
> > 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.
> 
> 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

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html