Re: [patch 05/25] ata: add the SW NCQ support to sata_nv for MCP51/MCP55/MCP61

[Jeff Garzik <jeff@garzik.org>]

akpm@linux-foundation.org wrote:
> From: Kuan Luo <kluo@nvidia.com>
> 
> Add the Software NCQ support to sata_nv.c for MCP51/MCP55/MCP61 SATA
> controller.  NCQ function is disable by default, you can enable it with
> 'swncq=1'.  NCQ will be turned off if the drive is Maxtor on MCP51 or MCP55
> rev 0xa2 platform.
> 
> [akpm@linux-foundation.org: build fix]
> Signed-off-by: Kuan Luo <kluo@nvidia.com>
> Signed-off-by: Peer Chen <pchen@nvidia.com>
> Cc: Zoltan Boszormenyi <zboszor@dunaweb.hu>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  drivers/ata/sata_nv.c |  860 +++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 851 insertions(+), 9 deletions(-)

I finally gave this a thorough review.

Overall, good work.  The state transitions all seem solid.  I made 
several minor changes and cleanups, and checked it into the 'nv-swncq' 
branch of 
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/libata-dev.git

Two hurdles before I'm ready to push upstream:

* someone please verify my minor changes did not break anything; I don't 
have real hardware

* the pp->lock appears unnecessary at best, wrong at worst.  needs 
additional analysis.