Jeff Garzik írta:
Zoltan Boszormenyi wrote:
Jeff Garzik írta:
Zoltan Boszormenyi wrote:
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,
that's a bug. fixed.
I also applied your cleanup.
Thanks, but you took the wrong cleanup patch.
My original, i.e. this chunk below is wrong.
@@ -2291,8
<http://git.kernel.org/?p=linux/kernel/git/jgarzik/libata-dev.git;a=blob;f=drivers/ata/sata_nv.c;h=540f218e10e56c8a85a937bc920a5d96434bcc8f;hb=41f0f8af3edfccd58b5c9d714928d773a072b693#l2291>
+2283,7
<http://git.kernel.org/?p=linux/kernel/git/jgarzik/libata-dev.git;a=blob;f=drivers/ata/sata_nv.c;h=ffd00f5c533d7b58dde67cfef7d2191ca15ea140;hb=5a5a9e1890b8260686218a68862d880daee1a817#l2283>
@@ static void nv_swncq_host_interrupt(struct ata_port *ap, u16 fis)
*/
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;
It should be a binary OR between the flags. This caused immediate
NCQ errors upon boot and lead to disabling NCQ.
My second patch has it corrected.
fixed
Thanks. And would you please also enable swncq by default? :-)
It's very stable.
Best regards,
Zoltán Böszörményi
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
