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

[Gilbert Wu <Gilbert_Wu@adaptec.com>]

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