Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Kurt Garloff <garloff@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 842 bytes --]

Guennadi,

I have no clue what's going wrong yet with INQUIRY. I believe it has
to do with some pci mapping we get wrong. 

I deferred the pci mapping to StartSCSI, so we don't create mappings
for queued SRBs. Some hardware may have IOMMUs and will have limited
sizes available.

It also allows to clean up some code as we can rely on a mapping being
created if needed upon StartSCSI. Likewise, I moved pci mapping cleanup
to SRBdone. Every command should go through here.
I still need to check the error handling paths, though.

Patch is on top of yours.

Likely, it won't help with the INQUIRY problem :(
I still have to get a machine with the DC390 up again.

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #1.2: tmscsim-pcimap.diff --]
[-- Type: text/plain, Size: 9332 bytes --]

--- drivers/scsi/tmscsim.c.guennadi	2003-11-18 10:15:30.000000000 +0100
+++ drivers/scsi/tmscsim.c	2003-11-18 10:24:08.232277272 +0100
@@ -970,6 +970,72 @@
     }
 }
 
+
+/* Create pci mapping */
+static int dc390_pci_map (PSRB pSRB)
+{
+	int error = 0;
+	Scsi_Cmnd *pcmd = pSRB->pcmd;
+	struct pci_dev *pdev = pSRB->pSRBDCB->pDCBACB->pdev;
+	dc390_cmd_scp_t* cmdp = ((dc390_cmd_scp_t*)(&pcmd->SCp));
+	/* Map sense buffer */
+	if (pSRB->SRBFlag | AUTO_REQSENSE) {
+		cmdp->saved_dma_handle = pci_map_page(pdev,
+				virt_to_page(pcmd->sense_buffer),
+				(unsigned long)pcmd->sense_buffer & ~PAGE_MASK,
+				sizeof(pcmd->sense_buffer),
+				DMA_FROM_DEVICE);
+		pSRB->Segmentx.length = sizeof(pcmd->sense_buffer);
+		pSRB->SGcount = 1;
+		pSRB->pSegmentList = (PSGL) &pSRB->Segmentx;
+	/* Make SG list */	
+	} else if (pcmd->use_sg) {
+		pSRB->pSegmentList = (PSGL) pcmd->request_buffer;
+		pSRB->SGcount = pci_map_sg(pdev, pSRB->pSegmentList,
+				pcmd->use_sg, 
+				scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+		/* TODO: error handling */
+		if (!pSRB->SGcount)
+			error = 1;
+	/* Map single segment */	
+	} else if (pcmd->request_buffer) {
+		struct page *page = virt_to_page(pcmd->request_buffer);
+		unsigned long off = (unsigned long)pcmd->request_buffer & ~PAGE_MASK;
+		cmdp->saved_dma_handle = pci_map_page(pdev, 
+				page, off, pcmd->request_bufflen,
+				scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+		/* TODO: error handling */
+		pSRB->Segmentx.length = pcmd->request_bufflen;
+		pSRB->SGcount = 1;
+		pSRB->pSegmentList = (PSGL) &pSRB->Segmentx;
+	/* No mapping !? */	
+    	} else
+		pSRB->SGcount = 0;
+	return error;
+}
+
+/* Remove pci mapping */
+static void dc390_pci_unmap (PSRB pSRB)
+{
+	Scsi_Cmnd* pcmd = pSRB->pcmd;
+	struct pci_dev *pdev = pSRB->pSRBDCB->pDCBACB->pdev;
+	dc390_cmd_scp_t* cmdp = ((dc390_cmd_scp_t*)(&pcmd->SCp));
+
+	if (pSRB->SRBFlag) {
+		pci_unmap_page(pdev, cmdp->saved_dma_handle,
+			       sizeof(pcmd->sense_buffer), DMA_FROM_DEVICE);
+	} else if (pcmd->use_sg) {
+		pci_unmap_sg(pdev, pcmd->request_buffer, pcmd->use_sg,
+			     scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+	} else if (pcmd->request_buffer) {
+		pci_unmap_page(pdev,
+			       cmdp->saved_dma_handle,
+			       pcmd->request_bufflen,
+			       scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+	}
+}
+
+
 /***********************************************************************
  * Function: static void dc390_BuildSRB (Scsi_Cmd *pcmd, PDCB pDCB, 
  * 					 PSRB pSRB)
@@ -986,26 +1052,7 @@
     //pSRB->ScsiCmdLen = pcmd->cmd_len;
     //memcpy (pSRB->CmdBlock, pcmd->cmnd, pcmd->cmd_len);
     
-		printk("%s(): direction %d\n", __FUNCTION__, pcmd->sc_data_direction);
-    if( pcmd->use_sg )
-    {
-	pSRB->pSegmentList = (PSGL) pcmd->request_buffer;
-	pSRB->SGcount = pci_map_sg(pdev, pSRB->pSegmentList,
-				   pcmd->use_sg, scsi_to_pci_dma_dir(pcmd->sc_data_direction));
-    }
-    else if( pcmd->request_buffer )
-    {
-	struct page *page = virt_to_page(pcmd->request_buffer);
-	unsigned long off = (unsigned long)pcmd->request_buffer & ~PAGE_MASK;
-	((dc390_cmd_scp_t*)(&pcmd->SCp))->saved_dma_handle = pci_map_page(pdev, page, off,
-									  pcmd->request_bufflen,
-									  scsi_to_pci_dma_dir(pcmd->sc_data_direction));
-	pSRB->Segmentx.length = pcmd->request_bufflen;
-	pSRB->SGcount = 1;
-	pSRB->pSegmentList = (PSGL) &pSRB->Segmentx;
-    }
-    else
-	pSRB->SGcount = 0;
+	printk("%s(): direction %d\n", __FUNCTION__, pcmd->sc_data_direction);
 
     pSRB->SGIndex = 0;
     pSRB->AdaptStatus = 0;
@@ -1024,6 +1071,7 @@
     pSRB->ScsiPhase = 0;
     pSRB->EndMessage = 0;
     pSRB->TagNumber = 255;
+    /* KG: deferred PCI mapping to dc390_StartSCSI */
 }
 
 /* Put cmnd from Query to Waiting list and send next Waiting cmnd */
--- drivers/scsi/scsiiom.c.guennadi	2003-11-18 10:15:30.000000000 +0100
+++ drivers/scsi/scsiiom.c	2003-11-18 10:20:55.350599744 +0100
@@ -36,6 +36,9 @@
 	DEBUG0(printk ("DC390: We were just reset and don't accept commands yet!\n"));
 	return 1;
     }
+    /* KG: Moved pci mapping here */
+    dc390_pci_map(pSRB);
+    /* TODO: error handling */
     DC390_write8 (Scsi_Dest_ID, pDCB->TargetID);
     DC390_write8 (Sync_Period, pDCB->SyncPeriod);
     DC390_write8 (Sync_Offset, pDCB->SyncOffset);
@@ -189,19 +192,7 @@
 	    if (!ctr) printk (KERN_CRIT "DC390: dma_intr: DMA aborted unfinished: %06x bytes remain!!\n", DC390_read32 (DMA_Wk_ByteCntr));
 	    else /*if (!residual)*/
 	    {
-		Scsi_Cmnd* pcmd = pSRB->pcmd;
-		if (pcmd->use_sg)
-		{
-			pci_unmap_sg(pACB->pdev, pcmd->request_buffer, pcmd->use_sg,
-				     scsi_to_pci_dma_dir(pcmd->sc_data_direction));
-		}
-		else if (pcmd->request_buffer)
-		{
-			pci_unmap_page(pACB->pdev,
-				       ((dc390_cmd_scp_t*)(&pcmd->SCp))->saved_dma_handle,
-				       pcmd->request_bufflen,
-				       scsi_to_pci_dma_dir(pcmd->sc_data_direction));
-		}
+		    /* Done: Moved pci unmap to dc390_SRBdone */
 	    }
 	    /* residual =  ... */
 	  }
@@ -747,14 +738,17 @@
     struct pci_dev *pdev = pACB->pdev;
     pSRB->TotalXferredLen = 0;
     pSRB->SGIndex = 0;
-    if( pcmd->use_sg )
-    {
+    if (pcmd->use_sg) {
 	pSRB->pSegmentList = (PSGL) pcmd->request_buffer;
 	psgl = pSRB->pSegmentList;
+	/*
 	pci_unmap_sg(pdev, psgl, pcmd->use_sg,
 		     scsi_to_pci_dma_dir(pcmd->sc_data_direction));
 	pSRB->SGcount = pci_map_sg(pdev, psgl,
 				   pcmd->use_sg, scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+	 */
+	//dc390_pci_sync(pSRB);
+
 	while (pSRB->TotalXferredLen + (ULONG) psgl->length < pSRB->Saved_Ptr)
 	{
 	    pSRB->TotalXferredLen += (ULONG) psgl->length;
@@ -773,11 +767,12 @@
 	pSRB->SGBusAddr += (pSRB->Saved_Ptr - pSRB->TotalXferredLen);
 	printk (KERN_INFO "DC390: Pointer restored. Segment %i, Total %li, Bus %08lx\n",
 		pSRB->SGIndex, pSRB->Saved_Ptr, pSRB->SGBusAddr);
-    }
-    else if( pcmd->request_buffer )
-    {
+
+    } else if( pcmd->request_buffer ) {
+   
 	struct page *page = virt_to_page(pcmd->request_buffer + pSRB->Saved_Ptr);
 	unsigned long off = (unsigned long)(pcmd->request_buffer + pSRB->Saved_Ptr) & ~PAGE_MASK;
+	/*
 	dc390_cmd_scp_t* cmdp = ((dc390_cmd_scp_t*)(&pcmd->SCp));
 
 	if (cmdp->saved_dma_handle)
@@ -787,23 +782,23 @@
 	cmdp->saved_dma_handle = pci_map_page(pdev, page, off,
 					      pcmd->request_bufflen - pSRB->Saved_Ptr,
 					      scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+	 */
+	//dc390_pci_sync(pSRB);
 
 	pSRB->Segmentx.length = pcmd->request_bufflen - pSRB->Saved_Ptr;
 	pSRB->SGcount = 1;
 	pSRB->pSegmentList = (PSGL) &pSRB->Segmentx;
-    }
-     else
-       {
+    } else {
 	 pSRB->SGcount = 0;
 	 printk (KERN_INFO "DC390: RESTORE_PTR message for Transfer without Scatter-Gather ??\n");
-       };
+    };
 
   pSRB->TotalXferredLen = pSRB->Saved_Ptr;
 };
 
 
 /* According to the docs, the AM53C974 reads the message and 
- * generates a Succesful Operation IRQ before asserting ACK for
+ * generates a Successful Operation IRQ before asserting ACK for
  * the last byte (how does it know whether it's the last ?) */
 /* The old code handled it in another way, indicating, that on
  * every message byte an IRQ is generated and every byte has to
@@ -1395,6 +1390,9 @@
     PSGL   ptr2;
     ULONG  swlval;
 
+    /* KG: Moved pci_unmap here */
+    dc390_pci_unmap(pSRB);
+
     pcmd = pSRB->pcmd; DCB_removed = 0;
     status = pSRB->TargetStatus;
     ptr = (PSCSI_INQDATA) (pcmd->request_buffer);
@@ -1468,6 +1466,7 @@
 	    pSRB->SGIndex = 0;
 	    pSRB->TotalXferredLen = 0;
 	    pSRB->SGToBeXferLen = 0;
+	    /*
 	    if( pcmd->use_sg ) {
 		pSRB->pSegmentList = (PSGL) pcmd->request_buffer;
 		pSRB->SGcount = pci_map_sg(pACB->pdev, pSRB->pSegmentList,
@@ -1484,6 +1483,8 @@
 										  scsi_to_pci_dma_dir(pcmd->sc_data_direction));
 		pSRB->Segmentx.length = pcmd->request_bufflen;
 	    }
+	    */
+
 	    if( dc390_StartSCSI( pACB, pDCB, pSRB ) ) {
 		dc390_Going_to_Waiting ( pDCB, pSRB );
 		dc390_waiting_timer (pACB, HZ/5);
@@ -1551,6 +1552,7 @@
 		pSRB->SGIndex = 0;
 		pSRB->TotalXferredLen = 0;
 		pSRB->SGToBeXferLen = 0;
+		/*
 		if( pcmd->use_sg )
 		{
 		    pSRB->pSegmentList = (PSGL) pcmd->request_buffer;
@@ -1567,6 +1569,7 @@
 										      scsi_to_pci_dma_dir(pcmd->sc_data_direction));
 		    pSRB->Segmentx.length = pcmd->request_bufflen;
 		}
+		*/
 		if( dc390_StartSCSI( pACB, pDCB, pSRB ) ) {
 		    dc390_Going_to_Waiting ( pDCB, pSRB );
 		    dc390_waiting_timer (pACB, HZ/5);
@@ -1805,6 +1808,9 @@
 
     pcmd = pSRB->pcmd;
 
+    /* We are called from SRBdone, original PCI mapping has been removed
+     * already, new one is set up from StartSCSI */
+    /*
     ((dc390_cmd_scp_t*)(&pcmd->SCp))->saved_dma_handle = pci_map_page(pACB->pdev,
 								      virt_to_page(pcmd->sense_buffer),
 								      (unsigned long)pcmd->sense_buffer & ~PAGE_MASK,
@@ -1813,6 +1819,7 @@
     pSRB->Segmentx.length = sizeof(pcmd->sense_buffer);
     pSRB->pSegmentList = &pSRB->Segmentx;
     pSRB->SGcount = 1;
+    */
     pSRB->SGIndex = 0;
 
     //pSRB->CmdBlock[0] = REQUEST_SENSE;

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]