Add Promise SuperTrak EX 'stex' driver

Adds the 'stex' driver for Promise SuperTrak EX storage controllers. These controllers present themselves as SCSI, though like 3ware, megaraid and others, the underlying storage may or may not be SCSI.

As discussed, the block tagging stuff is a post-merge todo item.

drivers/scsi/stex.c new file +9 -0

+static int stex_reset(struct scsi_cmnd *cmd)
+{
+	struct st_hba *hba;
+	int tag;
+	int i = 0;
+	unsigned long flags;
+	hba = (struct st_hba *) &cmd->device->host->hostdata[0];
+
+wait_cmds:

drivers/scsi/stex.c new file +18 -0

+static void stex_hba_stop(struct st_hba *hba)
+{
+	unsigned long flags;
+	int i;
+	u16 tag;
+
+	spin_lock_irqsave(hba->host->host_lock, flags);
+	if ((tag = stex_alloc_tag((unsigned long *)&hba->tag))
+		== TAG_BITMAP_LENGTH) {
+		spin_unlock_irqrestore(hba->host->host_lock, flags);
+		printk(KERN_ERR DRV_NAME "(%s): unable to alloc tag\n",
+			pci_name(hba->pdev));
+		return;
+	}
+	for (i=0; i<(ST_MAX_ARRAY_SUPPORTED*ST_MAX_LUN_PER_TARGET*2); i++) {
+		stex_internal_flush(hba, i, tag);
+		spin_unlock_irqrestore(hba->host->host_lock, flags);
+

Quoted 4 lines · Jeff Garzik

Adds the 'stex' driver for Promise SuperTrak EX storage controllers. These controllers present themselves as SCSI, though like 3ware, megaraid and others, the underlying storage may or may not be SCSI.

Quoted 1 line

As discussed, the block tagging stuff is a post-merge todo item.

That's not exactly my recollection of the discussion: I thought we were still discussing the chicken and egg issue (which is we have APIs to do this per host tagging which stex duplicates on the grounds no-one's using the current APIs). Jens and I seem to be in agreement that stex should try the API's and well make any changes that become necessary to block or SCSI happen.

Quoted 10 lines

(quoted diff) +1 -0

+	switch (cmd->cmnd[0]) {

Review

Quoted 9 lines

(quoted diff) +9 -0

+	case MODE_SENSE_10:
+	{
+		static char mode_sense10[8] = { 0, 6, 0, 0, 0, 0, 0, 0 };
+
+		stex_direct_copy(cmd, mode_sense10, sizeof(mode_sense10));
+		cmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8;
+		done(cmd);
+		return 0;
+	}

This looks like it will trick the sd driver into reading uninitialised data for the drive caching parameters ... there are obviously faults on both sides, but I think when you ask for a mode page and you get a success return, you're entitled to think you got it ...

Quoted 10 lines

(quoted diff) +1 -0

+	case INQUIRY:

Review

Quoted 9 lines

(quoted diff) +9 -0

+		if (id != ST_MAX_ARRAY_SUPPORTED)
+			break;
+		if (lun == 0 && (cmd->cmnd[1] & INQUIRY_EVPD) == 0) {
+			stex_direct_copy(cmd, console_inq_page,
+				sizeof(console_inq_page));
+			cmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8;
+		} else
+			cmd->result = DID_ERROR << 16 | COMMAND_COMPLETE << 8;
+		done(cmd);

The error return isn't correct you should never use DID_ERROR for an uncorrectable error because it will cause a retry (which you'll fail again). For an unsupported inquiry the correct return should be Check Condition/Illegal Request/Invalid Field in CDB

Quoted 5 lines

Quoted 5 lines

(quoted diff) +5 -0

+	case INQUIRY:
+		if (id != ST_MAX_ARRAY_SUPPORTED)
+			break;
+		if (lun == 0 && (cmd->cmnd[1] & INQUIRY_EVPD) == 0) {
+			stex_direct_copy(cmd, console_inq_page,

Quoted 5 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+				sizeof(console_inq_page));
+			cmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8;
+		} else
+			cmd->result = DID_ERROR << 16 | COMMAND_COMPLETE << 8;
+		done(cmd);

The error return here looks like it shouldn't be DID_ERROR either. I assume the error is a format one and uncorrectable by a retry?

Quoted 4 lines

(quoted diff) +4 -0

+	}
+
+	hba->dma_mem = pci_alloc_consistent(pdev,
+		STEX_BUFFER_SIZE, &hba->dma_handle);

Review

Quoted 6 lines

(quoted diff) +6 -0

+	if (!hba->dma_mem) {
+		err = -ENOMEM;
+		printk(KERN_ERR DRV_NAME "(%s): dma mem alloc failed\n",
+			pci_name(pdev));
+		goto out_iounmap;
+	}

This should be dma_alloc_coherent, not pci_alloc_consistent.

Quoted 5 lines · James Bottomley

Quoted 4 lines · Jeff Garzik

Adds the 'stex' driver for Promise SuperTrak EX storage controllers. These controllers present themselves as SCSI, though like 3ware, megaraid and others, the underlying storage may or may not be SCSI.

Quoted 8 lines

Quoted 1 line

As discussed, the block tagging stuff is a post-merge todo item.

That's not exactly my recollection of the discussion: I thought we were still discussing the chicken and egg issue (which is we have APIs to do this per host tagging which stex duplicates on the grounds no-one's using the current APIs). Jens and I seem to be in agreement that stex should try the API's and well make any changes that become necessary to block or SCSI happen.

Please re-read the end of the thread. The last word was "ok, let's go ahead and get this merged."

Quoted 2 lines

The error return here looks like it shouldn't be DID_ERROR either. I assume the error is a format one and uncorrectable by a retry?

There is only one 'case INQUIRY' in the entire driver, so I assume you accidentally responded to the same code segment twice.

Quoted 1 line

This should be dma_alloc_coherent, not pci_alloc_consistent.

This is perfectly normal and proper in a PCI-only driver. pci_xxx is not a deprecated API, it is a convenience API.

Using dma_xxx only causes needless work.

However, I'll take on some of this ... I'll convert the aic7xxx driver which is our current shared host tag driver ... then you only need copy it to do stex.

No, sorry, misquoted ... the above comment applies to the case PASSTHRU_CMD, which has the same problem (it would repeat a malformed command).

Quoted 4 lines

Quoted 1 line

This should be dma_alloc_coherent, not pci_alloc_consistent.

This is perfectly normal and proper in a PCI-only driver. pci_xxx is not a deprecated API, it is a convenience API.

Using dma_xxx only causes needless work.

What work? it's an exact drop in replacement. However, the only usage

of pci_xxx I'm requiring to be fixed is the pci_alloc_consistent, primarily because pci_alloc_consistent *is* deprecated: it forces a GFP_ATOMIC allocation of a potentially large amount of data. dma_alloc_coherent allows you to specify gfp flags, which, in this case, should be GFP_KERNEL.