Hi,
Jeff Garzik írta:
akpm@xxxxxxxxxxxxxxxxxxxx wrote:
From: Kuan Luo <kluo@xxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxx: build fix]
Signed-off-by: Kuan Luo <kluo@xxxxxxxxxx>
Signed-off-by: Peer Chen <pchen@xxxxxxxxxx>
Cc: Zoltan Boszormenyi <zboszor@xxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
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
After reading the diff between the original and your cleaned up version
it seems both the change from 4 individual flags to a single integer and the
nv_swncq_bmdma_stop() -> __ata_bmdma_stop() transition are obviously
correct.
I attached a small cleanup patch which may make one check a bit more
readable.
However, can you explain this chunk below? Why isn't it needed?
@@ -615,7 +622,6 @@ static const struct ata_port_info nv_por
{
.sht = &nv_swncq_sht,
.flags = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
- .link_flags = ATA_LFLAG_HRST_TO_RESUME,
.pio_mask = NV_PIO_MASK,
.mwdma_mask = NV_MWDMA_MASK,
.udma_mask = NV_UDMA_MASK,
* the pp->lock appears unnecessary at best, wrong at worst. needs
additional analysis.
I will test the code without this locking. Can you give me a better idea
besides beating both my disks with separate requests? Say, bonnie++
and simultaneous hdparm -tT on both?
Best regards,
Zoltán Böszörményi
--- sata_nv.c.committed 2007-09-21 06:49:23.000000000 +0200
+++ sata_nv.c.cleanup-swncq 2007-09-21 07:47:24.000000000 +0200
@@ -2290,8 +2290,7 @@
*/
pp->dhfis_bits |= (0x1 << pp->last_issue_tag);
pp->ncq_flags |= ncq_saw_d2h;
- if ((pp->ncq_flags & ncq_saw_sdb) ||
- (pp->ncq_flags & ncq_saw_backout)) {
+ if (pp->ncq_flags & (ncq_saw_sdb || ncq_saw_backout)) {
ata_ehi_push_desc(ehi, "illegal fis transaction");
ehi->err_mask |= AC_ERR_HSM;
ehi->action |= ATA_EH_HARDRESET;
