Re: SATA status reports update

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[cc'ing linux-ide]

Robert Hancock wrote:
Jeff Garzik wrote:
Robert Hancock wrote:
Jeff Garzik wrote:
Prakash Punnoor wrote:
Well, how would one debug it w/o hw docs? Or is it possible to compare the patch with a working driver for another chipset?

Well, it is based off of the standard ADMA[1] specification, albeit with modifications. There is pdc_adma.c, which is also based off ADMA. And the author (from NVIDIA) claims that the driver worked at one time, so maybe it is simply bit rot that broke the driver.

If I knew the answer, it would be fixed, so the best answer unfortunately is "who knows".

I wish I had the time. But I also wish I had a team of programmers working on libata, too ;-)

Do you know exactly what is allegedly broken in that version? I see that there are some functions which are just "TODO"..

I just know it was a working driver at one time.

I had a look at the ADMA patch. It looks like it is vaguely based off the ADMA spec, though with some significant changes (i.e. 64-bit addresses instead of 32-bit, some things are missing or at least not defined in the constants provided in the patch).

I think the code will more or less work in ADMA mode with NCQ disabled (i.e. how it is in the patch currently, with #define NV_ADMA_NCQ commented out). However, with NCQ on there would be a few problems:

-When the driver gets a command which is not DMA-mapped (i.e. PIO commands), it switches the controller from ADMA mode into port-register mode and then issues the command in the existing fashion. This isn't going to work very well if there are already NCQ command(s) in progress, which I assume is a possibility. Either the driver needs to stall the PIO command until all the NCQ commands are done and prevent any other NCQ commands starting while the PIO is in progress (is this viable?), or it needs to push the PIO command through the ADMA pipeline.

Actually, libata core layer already does it. It never mixes NCQ and non-NCQ commands. sata_nv can safely assume that those two sets of commands are always issued disjointly. The relevant function is libata-scsi.c::ata_scmd_need_defer().

The ADMA standard provides a means for executing PIO commands through the pipeline using PIO-over-DMA, but there's not enough info to say whether the NVIDIA controller implements that the same way or at all. Jeff, you may be able to help with this if you have access to the docs.

It would be nice to have that but I'm doubtful it would worth the effort. I would just leave it as it is as long as it works.

-Inside the interrupt handler the driver uses ata_qc_from_tag(ap, ap->active_tag) to find the qc which was just completed. This won't work in NCQ as active_tag is not used and multiple commands may be in progress. It should be checking the CPB flags on all the active CPBs to see which one(s) have completed (or maybe the hardware has a register that indicates which CPBs have been completed already, the patch doesn't provide a hint of how that would work however).

So it looks like it needs some work before NCQ will work properly. However, there would be some gains to getting ADMA working even without NCQ, both in terms of reduced CPU overhead. Also, ADMA supports full 64-bit DMA as opposed to the 32-bit DMA capability of the standard interface, which would reduce IOMMU load on systems with RAM above 4GB. (Note that this is broken in the patch currently, the sg addresses get dumped into a u32 and truncated before they are written to the controller, and it also doesn't set a 64-bit DMA mask in ADMA mode..)

Not only that, hopefully, it will show better EH behavior. sata_nv's TF register mode sometimes hangs holding PCI bus (as in IORDY lockup). This happens a lot if you pull a disk out while it's actively processing a command but doesn't seem to be restricted to that. Also, it has been suggested that sata_nv's TF register mode might involve some nasty SMM code. I don't recall whether it was verified tho.

Anyways, it would be very nice to have working nv_adma. I have a CK804 nv but it's my primary work machine and I'm too lazy to develop on it, but I would be more than happy to test or answer questions.

Thanks.

--
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux