Messages in this thread | | | Date | Thu, 26 May 2005 19:11:33 +0200 | From | Jens Axboe <> | Subject | Re: Playing with SATA NCQ |
| |
On Thu, May 26 2005, Jens Axboe wrote: > On Thu, May 26 2005, Jeff Garzik wrote: > > > static void ahci_qc_prep(struct ata_queued_cmd *qc) > > > { > > > struct ahci_port_priv *pp = qc->ap->private_data; > > >- u32 opts; > > >+ void *port_mmio = (void *) qc->ap->ioaddr.cmd_addr; > > > const u32 cmd_fis_len = 5; /* five dwords */ > > >+ dma_addr_t cmd_tbl_dma; > > >+ u32 opts; > > >+ int offset; > > >+ > > >+ if (qc->flags & ATA_QCFLAG_NCQ) { > > >+ pp->sactive |= (1 << qc->tag); > > >+ > > >+ writel(1 << qc->tag, port_mmio + PORT_SCR_ACT); > > >+ readl(port_mmio + PORT_SCR_ACT); /* flush */ > > >+ } > > > > Wrong, you should do this in ahci_qc_issue not here. > > Are you sure, I moved this on purpose? I think the reason I did this was > the wording at the back of the the sata-ii spec (appendix b) that says > something ala 'preset the active bit and transmit a register FIS'. Feel > free to point me at the authoritative wording in the ACHI spec. > > One thing that I definitely think _was_ wrong with the sactive bit > before, is that you set it unconditionally of whether this was an NCQ > command or not. The maxtor drives don't clear sactive on non-fpdma > commands, which confused me at first.
Re-reading AHCI spec, it does indicate that you want to set SActive after building the command. I'll move it back, but keep the conditional of setting SActive on queued commands.
-- Jens Axboe
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |