Re: Playing with SATA NCQ

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

 



Jens Axboe wrote:
On Thu, May 26 2005, Jeff Garzik wrote:
+int ata_read_log_page(struct ata_port *ap, unsigned int device, char page,
+		      char *buffer, unsigned int sectors)
+{
+	struct ata_device *dev = &ap->device[device];
+	DECLARE_COMPLETION(wait);
+	struct ata_queued_cmd *qc;
+	unsigned long flags;
+	u8 status;
+	int rc;
+
+	assert(dev->class == ATA_DEV_ATA);
+
+	ata_dev_select(ap, device, 1, 1);

is this needed? These types of calls need to be removed, in general, as they don't make sense on FIS-based hardware at all.


You tell me, this read_log_page() was mainly copy-pasted from the pio
driven function above it. I'll try and kill the select when doing error
testing.


+	printk("RLP issue\n");
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+	rc = ata_qc_issue(qc);
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+	printk("RLP issue done\n");
+
+	if (rc)
+		return -EIO;
+
+	wait_for_completion(&wait);
+
+	printk("RLP wait done\n");
+
+	status = ata_chk_status(ap);
+	if (status & (ATA_ERR | ATA_ABORTED))
+		return -EIO;

we need to get rid of this too for AHCI-like devices


Can you expand on that?

(this covers both quoted questions above)

The PIO function assumes that PCI IDE-like ATA register blocks (command registers, control registers) are available. The read-log-page function can make no such assumptions.

dev-select and check-status should both be done by the machinery that occurs once you start things in motion by calling ata_qc_issue().

Doing things this way is necessary for FIS-based hardware like AHCI or SiI 3124.


#ifdef CONFIG_PCI
EXPORT_SYMBOL_GPL(pci_test_config_bits);
Index: drivers/scsi/libata-scsi.c
===================================================================
--- f5c58b6b0cfd2a92fb3b1d1f4cbfdfb3df6f45d6/drivers/scsi/libata-scsi.c (mode:100644)
+++ uncommitted/drivers/scsi/libata-scsi.c  (mode:100644)
@@ -336,6 +336,7 @@
	if (sdev->id < ATA_MAX_DEVICES) {
		struct ata_port *ap;
		struct ata_device *dev;
+		int depth;

		ap = (struct ata_port *) &sdev->host->hostdata[0];
		dev = &ap->device[sdev->id];
@@ -353,6 +354,13 @@
			 */
			blk_queue_max_sectors(sdev->request_queue, 2048);
		}
+
+		if (dev->flags & ATA_DFLAG_NCQ) {
+			int ddepth = ata_id_queue_depth(dev->id) + 1;
+
+			depth = min(sdev->host->can_queue, ddepth);
+ scsi_adjust_queue_depth(sdev, MSG_ORDERED_TAG, depth);

For all hardware that uses SActive (all NCQ), the max is 31 not 32.


That's not true, the max is 32 counting 0 as a valid tag. So 31 is
indeed th max tag value, but 32 is the depth.

I was talking about depth. In libata, it's a policy decision to never use more than 31 tags at any given time.

You can change it from 31 to 32 in SuSE for value-add, if you wish :)

Note also that error handling occasionally needs a command slot, so the limit may even be 30 (or 31 at most).



The two depths were added because we need to differentiate between the
two for issuing new commands. ncq_depth > 0 is fine for issuing a new
FPDMA request, where as non-FPDMA commands need both !ncq_depth and
!depth.

You can definitely handle both FPDMA and non-FPDMA with a single variable. Think harder on this one. You have flags to work with, you know...

	Jeff


-
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