Skip to contentSkip to conversation

qla4xxx: Remove funcs with no callers in ql4_init.c

PATCH In review Doug Maxey 4 participants 11 messages Aug 5, 2006 – Aug 10, 2006 1 message not in the archive

Subsystems (2)
SCSI QLogic QLA4XXX iSCSI driver
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Doug Maxey

author
#1 lore raw

PATCH 1/4 qla4xxx: Remove funcs with no callers in ql4_init.c

From: Doug Maxey <[email protected]>

These can be added later when a caller exists.
qla4xxx_login_device()
qla4xxx_logout_device()
qla4xxx_delete_device()

Signed-off-by Doug Maxey <[email protected]>

---

 drivers/scsi/qla4xxx/ql4_init.c |  163 +--------------------------------------
 1 files changed, 5 insertions(+), 158 deletions(-)
drivers/scsi/qla4xxx/ql4_init.c +5 -158
index 1e55e42..8608106 100644
@@ -41,7 +41,7 @@ static void ql4xxx_set_mac_number(struct
 }
 
 /**
- * qla4xxx_free_ddb - deallocate ddb	
+ * qla4xxx_free_ddb - deallocate ddb
  * @ha: pointer to host adapter structure.
  * @ddb_entry: pointer to device database entry
  *

@@ -376,7 +376,7 @@ static struct ddb_entry* qla4xxx_get_ddb
  *
  * This routine updates the driver's internal device database entry
  * with information retrieved from the firmware's device database
- * entry for the specified device. The ddb_entry->fw_ddb_index field 
+ * entry for the specified device. The ddb_entry->fw_ddb_index field
  * must be initialized prior to	calling this routine
  *
  **/

@@ -494,7 +494,7 @@ struct ddb_entry * qla4xxx_alloc_ddb(str
  * @ha: Pointer to host adapter structure.
  *
  * This routine searches for all valid firmware ddb entries and builds
- * an internal ddb list. Ddbs that are considered valid are those with 
+ * an internal ddb list. Ddbs that are considered valid are those with
  * a device state of SESSION_ACTIVE.
  **/
 static int qla4xxx_build_ddb_list(struct scsi_qla_host *ha)

@@ -540,7 +540,7 @@ static int qla4xxx_build_ddb_list(struct
 				qla4xxx_set_ddb_entry(ha, fw_ddb_index, 0);
 		}
 
-		if (ddb_state != DDB_DS_SESSION_ACTIVE) 
+		if (ddb_state != DDB_DS_SESSION_ACTIVE)
 			goto next_one;
 		/*
 		 * if fw_ddb with session active state found,

@@ -1134,7 +1134,7 @@ static int qla4xxx_start_firmware(struct
  * @renew_ddb_list: Indicates what to do with the adapter's ddb list
  *	after adapter recovery has completed.
  *	0=preserve ddb list, 1=destroy and rebuild ddb list
- * 
+ *
  * This routine parforms all of the steps necessary to initialize the adapter.
  *
  **/

@@ -1340,156 +1340,3 @@ int qla4xxx_process_ddb_changed(struct s
 	return QLA_SUCCESS;
 }
 
-/**
- * qla4xxx_login_device - login to target device
- * @ha: Pointer to host adapter structure.
- * @fw_ddb_index: Index of the device to login
- * @connection_id: Connection ID of the device to login
- *
- * This routine is called by the login IOCTL to log in the specified device.
- **/
-int qla4xxx_login_device(struct scsi_qla_host *ha, uint16_t fw_ddb_index,
-			 uint16_t connection_id)
-{
-	struct ddb_entry * ddb_entry;
-	int status = QLA_ERROR;
-
-	ddb_entry = qla4xxx_lookup_ddb_by_fw_index(ha, fw_ddb_index);
-	if (ddb_entry == NULL)
-		goto exit_login_device;
-
-	if (qla4xxx_get_fwddb_entry(ha, fw_ddb_index, NULL, 0, NULL, NULL,
-				    &ddb_entry->fw_ddb_device_state, NULL,
-				    NULL, NULL) == QLA_ERROR)
-		goto exit_login_device;
-
-	if (ddb_entry->fw_ddb_device_state == DDB_DS_SESSION_ACTIVE) {
-		status = QLA_SUCCESS;
-		goto exit_login_device;
-	}
-
-	if (qla4xxx_conn_close_sess_logout(ha, fw_ddb_index, connection_id,
-					   LOGOUT_OPTION_RELOGIN)
-	    != QLA_SUCCESS)
-		goto exit_login_device;
-
-	status = QLA_SUCCESS;
-
-exit_login_device:
-	return status;
-}
-
-/**
- * qla4xxx_logout_device - logout target device
- * @ha: Pointer to host adapter structure.
- * @fw_ddb_index: Index of the device to logout
- * @connection_id: Connection ID of the device to logout
- *
- * This support routine is called by the logout IOCTL to log out
- * the specified device.
- **/
-int qla4xxx_logout_device(struct scsi_qla_host *ha, uint16_t fw_ddb_index,
-			  uint16_t connection_id)
-{
-	int status = QLA_ERROR;
-	struct ddb_entry * ddb_entry;
-	uint32_t old_fw_ddb_device_state;
-
-	ddb_entry = qla4xxx_lookup_ddb_by_fw_index(ha, fw_ddb_index);
-	if (ddb_entry == NULL)
-		goto exit_logout_device;
-
-	if (qla4xxx_get_fwddb_entry(ha, fw_ddb_index, NULL, 0, NULL, NULL,
-				    &old_fw_ddb_device_state, NULL, NULL,
-				    NULL) != QLA_SUCCESS)
-		goto exit_logout_device;
-
-	set_bit(DF_NO_RELOGIN, &ddb_entry->flags);
-	if (qla4xxx_conn_close_sess_logout(ha, fw_ddb_index, connection_id,
-					   LOGOUT_OPTION_CLOSE_SESSION)
-	    != QLA_SUCCESS)
-		goto exit_logout_device;
-
-	status = QLA_SUCCESS;
-
-exit_logout_device:
-	return status;
-}
-
-/**
- * qla4xxx_delete_device - delete specified ddb entry
- * @ha: Pointer to host adapter structure.
- * @fw_ddb_index: Index of the device to delete
- * @connection_id: Connection ID of the device to delete
- *
- * This routine is called by the logout IOCTL to delete the specified
- * device. Send the LOGOUT and DELETE_DDB commands for the specified
- * target, even if it's not in our internal database.
- **/
-int qla4xxx_delete_device(struct scsi_qla_host *ha, uint16_t fw_ddb_index,
-			  uint16_t connection_id)
-{
-	int status = QLA_ERROR;
-	uint32_t fw_ddb_device_state = 0xFFFF;
-	u_long wait_count;
-	struct ddb_entry * ddb_entry;
-
-	/* If the device is in our internal tables, set the NO_RELOGIN bit. */
-	ddb_entry = qla4xxx_lookup_ddb_by_fw_index(ha, fw_ddb_index);
-	if (ddb_entry != NULL)
-		set_bit(DF_NO_RELOGIN, &ddb_entry->flags);
-
-	/*
-	 * If the device state is already one that we can delete, bypass the
-	 * logout command.
-	 */
-	qla4xxx_get_fwddb_entry(ha, fw_ddb_index, NULL, 0, NULL, NULL,
-				&fw_ddb_device_state, NULL, NULL, NULL);
-	if (fw_ddb_device_state == DDB_DS_UNASSIGNED ||
-	    fw_ddb_device_state == DDB_DS_NO_CONNECTION_ACTIVE ||
-	    fw_ddb_device_state == DDB_DS_SESSION_FAILED)
-		goto delete_ddb;
-
-	/* First logout index */
-	if (qla4xxx_conn_close_sess_logout(ha, fw_ddb_index, connection_id,
-					   LOGOUT_OPTION_CLOSE_SESSION) !=
-	    QLA_SUCCESS) {
-		DEBUG2(printk("scsi%ld: %s: LOGOUT_OPTION_CLOSE_SESSION "
-			      "failed index [%d]\n", ha->host_no, __func__,
-			      fw_ddb_index));
-		goto exit_delete_ddb;
-	}
-
-	/* Wait enough time to complete logout */
-	wait_count = jiffies + LOGOUT_TOV * HZ;
-	while (qla4xxx_get_fwddb_entry(ha, fw_ddb_index, NULL, 0, NULL, NULL,
-				       &fw_ddb_device_state, NULL, NULL, NULL)
-	       == QLA_SUCCESS) {
-		if (time_after_eq(jiffies, wait_count))
-			goto exit_delete_ddb;
-		if (fw_ddb_device_state == DDB_DS_UNASSIGNED ||
-		    fw_ddb_device_state == DDB_DS_NO_CONNECTION_ACTIVE ||
-		    fw_ddb_device_state == DDB_DS_SESSION_FAILED)
-			break;
-		udelay(50);
-	}
-
-delete_ddb:
-	/* Now delete index */
-	if (qla4xxx_clear_database_entry(ha, fw_ddb_index) == QLA_SUCCESS) {
-		status = QLA_SUCCESS;
-
-		if (!ddb_entry)
-			goto exit_delete_ddb;
-
-		atomic_set(&ddb_entry->state, DDB_STATE_DEAD);
-		DEBUG(printk("scsi%ld: %s: removing index %d.\n", ha->host_no,
-			     __func__, fw_ddb_index));
-		ha->fw_ddb_index_map[fw_ddb_index] =
-			(struct ddb_entry *)INVALID_ENTRY;
-	}
-
-exit_delete_ddb:
-	return status;
-
-}
author
#2 lore raw

PATCH 2/4 qla4xxx: Add a timeout period and return status from ql4xxx_lock_drvr_wait()

In reply to a message not in the archive

From: Doug Maxey <[email protected]>

Change the func to only wait for QL4_LOCK_DRVR_WAIT ms. Fix callers to use the return status.

Signed-off-by Doug Maxey <[email protected]>

---

 drivers/scsi/qla4xxx/ql4_init.c |   26 ++++++++++++++++----------
 1 files changed, 16 insertions(+), 10 deletions(-)
drivers/scsi/qla4xxx/ql4_init.c +16 -10
index 8608106..8fb39b5 100644
@@ -988,25 +988,29 @@ static int qla4xxx_start_firmware_from_f
 	return status;
 }
 
-static void ql4xxx_lock_drvr_wait(struct scsi_qla_host *a)
+static int ql4xxx_lock_drvr_wait(struct scsi_qla_host *a)
 {
-	int i = 0;
-	while (1) {
+#define QL4_LOCK_DRVR_WAIT (20000)
+#define QL4_LOCK_DRVR_SLEEP (10)
+	int i = QL4_LOCK_DRVR_WAIT;
+
+	while (i > 0) {
 		if (ql4xxx_lock_drvr(a) == 0) {
-			msleep(10);
-			if (!i) {
+			msleep(QL4_LOCK_DRVR_SLEEP);
+			if (i == QL4_LOCK_DRVR_WAIT) {
 				DEBUG2(printk("scsi%ld: %s: Waiting for "
 					      "Global Init Semaphore...n",
 					      a->host_no,
 					      __func__));
-				i++;
 			}
+			i -= QL4_LOCK_DRVR_SLEEP;
 		} else {
 			DEBUG2(printk("scsi%ld: %s: Global Init Semaphore "
-				      "acquired.n", a->host_no, __func__));
-			break;
+				      "acquired\n", a->host_no, __func__));
+			return QLA_SUCCESS;
 		}
 	}
+	return QLA_ERROR;
 }
 
 /**

@@ -1030,7 +1034,8 @@ static int qla4xxx_start_firmware(struct
 	if (is_qla4022(ha))
 		ql4xxx_set_mac_number(ha);
 
-	ql4xxx_lock_drvr_wait(ha);
+	if (ql4xxx_lock_drvr_wait(ha))
+		return QLA_ERROR;
 
 	spin_lock_irqsave(&ha->hardware_lock, flags);
 

@@ -1107,7 +1112,8 @@ static int qla4xxx_start_firmware(struct
 		config_chip = 1;
 
 		/* Reset clears the semaphore, so aquire again */
-		ql4xxx_lock_drvr_wait(ha);
+		if (ql4xxx_lock_drvr_wait(ha))
+			return QLA_ERROR;
 	}
 
 	if (config_chip) {
author
#3 lore raw

PATCH 3/4 qla4xxx: use dev_xxx on some pci/dma resource alloc warning and error printks

In reply to a message not in the archive

From: Doug Maxey <[email protected]>

Rework some of the printk's in the pci and dma resource allocations to
- use the appropriate dev_xxx macro.
- be non conditional

Signed-off-by Doug Maxey <[email protected]>

---

 drivers/scsi/qla4xxx/ql4_os.c |   46 +++++++++++++++--------------------------
 1 files changed, 17 insertions(+), 29 deletions(-)
drivers/scsi/qla4xxx/ql4_os.c +17 -29
index 1da7cf8..d88c84f 100644
@@ -688,7 +688,7 @@ static void qla4xxx_free_adapter(struct 
  * @ha: pointer to adapter structure
  *
  * This routines maps HBA's registers from the pci address space
- * into the kernel virtual address space for memory mapped i/o. 
+ * into the kernel virtual address space for memory mapped i/o.
  **/
 static int qla4xxx_iospace_config(struct scsi_qla_host *ha)
 {

@@ -700,15 +700,12 @@ static int qla4xxx_iospace_config(struct
 	pio_flags = pci_resource_flags(ha->pdev, 0);
 	if (pio_flags & IORESOURCE_IO) {
 		if (pio_len < MIN_IOBASE_LEN) {
-			ql4_printk(KERN_WARNING, ha,
-				   "Invalid PCI I/O region size (%s)...\n",
-				   pci_name(ha->pdev));
+			dev_warn(&ha->pdev->dev,
+				   "Invalid PCI I/O region size\n");
 			pio = 0;
 		}
 	} else {
-		ql4_printk(KERN_WARNING, ha,
-			   "region #0 not a PIO resource (%s)...\n",
-			   pci_name(ha->pdev));
+		dev_warn(&ha->pdev->dev, "region #0 not a PIO resource\n");
 		pio = 0;
 	}
 

@@ -718,23 +715,19 @@ static int qla4xxx_iospace_config(struct
 	mmio_flags = pci_resource_flags(ha->pdev, 1);
 
 	if (!(mmio_flags & IORESOURCE_MEM)) {
-		ql4_printk(KERN_ERR, ha,
-			   "region #0 not an MMIO resource (%s), aborting\n",
-			   pci_name(ha->pdev));
+		dev_err(&ha->pdev->dev,
+			   "region #0 not an MMIO resource, aborting\n");
 		goto iospace_error_exit;
 	}
 	if (mmio_len < MIN_IOBASE_LEN) {
-		ql4_printk(KERN_ERR, ha,
-			   "Invalid PCI mem region size (%s), aborting\n",
-			   pci_name(ha->pdev));
+		dev_err(&ha->pdev->dev,
+			   "Invalid PCI mem region size, aborting\n");
 		goto iospace_error_exit;
 	}
 
 	if (pci_request_regions(ha->pdev, DRIVER_NAME)) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Failed to reserve PIO/MMIO regions (%s)\n",
-			   pci_name(ha->pdev));
-
+		dev_warn(&ha->pdev->dev,
+			 "Failed to reserve PIO/MMIO regions\n");
 		goto iospace_error_exit;
 	}
 

@@ -742,10 +735,7 @@ static int qla4xxx_iospace_config(struct
 	ha->pio_length = pio_len;
 	ha->reg = ioremap(mmio, MIN_IOBASE_LEN);
 	if (!ha->reg) {
-		ql4_printk(KERN_ERR, ha,
-			   "cannot remap MMIO (%s), aborting\n",
-			   pci_name(ha->pdev));
-
+		dev_err(&ha->pdev->dev, "cannot remap MMIO, aborting\n");
 		goto iospace_error_exit;
 	}
 

@@ -782,7 +772,7 @@ void qla4xxx_config_dma_addressing(struc
 /**
  * qla4xxx_mem_alloc - allocates memory for use by adapter.
  * @ha: Pointer to host adapter structure
- * 
+ *
  * Allocates DMA memory for request and response queues. Also allocates memory
  * for srbs.
  **/

@@ -799,9 +789,8 @@ static int qla4xxx_mem_alloc(struct scsi
 	ha->queues = dma_alloc_coherent(&ha->pdev->dev, ha->queues_len,
 					&ha->queues_dma, GFP_KERNEL);
 	if (ha->queues == NULL) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Memory Allocation failed - queues.\n");
-
+		dev_warn(&ha->pdev->dev,
+			 "Memory Allocation failed - queues.\n");
 		goto mem_alloc_error_exit;
 	}
 	memset(ha->queues, 0, ha->queues_len);

@@ -836,9 +825,8 @@ static int qla4xxx_mem_alloc(struct scsi
 	ha->srb_mempool = mempool_create(SRB_MIN_REQ, mempool_alloc_slab,
 					 mempool_free_slab, srb_cachep);
 	if (ha->srb_mempool == NULL) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Memory Allocation failed - SRB Pool.\n");
-
+		dev_warn(&ha->pdev->dev,
+			 "Memory Allocation failed - SRB Pool.\n");
 		goto mem_alloc_error_exit;
 	}
 

@@ -1709,7 +1697,7 @@ static int qla4xxx_eh_bus_reset(struct s
  * @ddb_entry: Pointer to device database entry
  *
  * This routine issues either a warm or cold target reset to the
- * specified device. The caller must ensure that the ddb_entry pointer 
+ * specified device. The caller must ensure that the ddb_entry pointer
  * is valid before calling this routine.
  *
  **/
#4 lore raw

Re: [PATCH ver3 3/4] qla4xxx: use dev_xxx on some pci/dma resource alloc warning and error printks

Quoted 108 lines · Doug Maxey

From: Doug Maxey <[email protected]>

Rework some of the printk's in the pci and dma resource allocations to
- use the appropriate dev_xxx macro.
- be non conditional

Signed-off-by Doug Maxey <[email protected]>

---

 drivers/scsi/qla4xxx/ql4_os.c |   46 +++++++++++++++--------------------------
 1 files changed, 17 insertions(+), 29 deletions(-)
drivers/scsi/qla4xxx/ql4_os.c +14 -25
index 1da7cf8..d88c84f 100644
@@ -688,7 +688,7 @@ static void qla4xxx_free_adapter(struct 
  * @ha: pointer to adapter structure
  *
  * This routines maps HBA's registers from the pci address space
- * into the kernel virtual address space for memory mapped i/o. 
+ * into the kernel virtual address space for memory mapped i/o.
  **/
 static int qla4xxx_iospace_config(struct scsi_qla_host *ha)
 {

@@ -700,15 +700,12 @@ static int qla4xxx_iospace_config(struct
 	pio_flags = pci_resource_flags(ha->pdev, 0);
 	if (pio_flags & IORESOURCE_IO) {
 		if (pio_len < MIN_IOBASE_LEN) {
-			ql4_printk(KERN_WARNING, ha,
-				   "Invalid PCI I/O region size (%s)...\n",
-				   pci_name(ha->pdev));
+			dev_warn(&ha->pdev->dev,
+				   "Invalid PCI I/O region size\n");
 			pio = 0;
 		}
 	} else {
-		ql4_printk(KERN_WARNING, ha,
-			   "region #0 not a PIO resource (%s)...\n",
-			   pci_name(ha->pdev));
+		dev_warn(&ha->pdev->dev, "region #0 not a PIO resource\n");
 		pio = 0;
 	}
 

@@ -718,23 +715,19 @@ static int qla4xxx_iospace_config(struct
 	mmio_flags = pci_resource_flags(ha->pdev, 1);
 
 	if (!(mmio_flags & IORESOURCE_MEM)) {
-		ql4_printk(KERN_ERR, ha,
-			   "region #0 not an MMIO resource (%s), aborting\n",
-			   pci_name(ha->pdev));
+		dev_err(&ha->pdev->dev,
+			   "region #0 not an MMIO resource, aborting\n");
 		goto iospace_error_exit;
 	}
 	if (mmio_len < MIN_IOBASE_LEN) {
-		ql4_printk(KERN_ERR, ha,
-			   "Invalid PCI mem region size (%s), aborting\n",
-			   pci_name(ha->pdev));
+		dev_err(&ha->pdev->dev,
+			   "Invalid PCI mem region size, aborting\n");
 		goto iospace_error_exit;
 	}
 
 	if (pci_request_regions(ha->pdev, DRIVER_NAME)) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Failed to reserve PIO/MMIO regions (%s)\n",
-			   pci_name(ha->pdev));
-
+		dev_warn(&ha->pdev->dev,
+			 "Failed to reserve PIO/MMIO regions\n");
 		goto iospace_error_exit;
 	}
 

@@ -742,10 +735,7 @@ static int qla4xxx_iospace_config(struct
 	ha->pio_length = pio_len;
 	ha->reg = ioremap(mmio, MIN_IOBASE_LEN);
 	if (!ha->reg) {
-		ql4_printk(KERN_ERR, ha,
-			   "cannot remap MMIO (%s), aborting\n",
-			   pci_name(ha->pdev));
-
+		dev_err(&ha->pdev->dev, "cannot remap MMIO, aborting\n");
 		goto iospace_error_exit;
 	}
 

@@ -782,7 +772,7 @@ void qla4xxx_config_dma_addressing(struc
 /**
  * qla4xxx_mem_alloc - allocates memory for use by adapter.
  * @ha: Pointer to host adapter structure
- * 
+ *
  * Allocates DMA memory for request and response queues. Also allocates memory
  * for srbs.
  **/

@@ -799,9 +789,8 @@ static int qla4xxx_mem_alloc(struct scsi
 	ha->queues = dma_alloc_coherent(&ha->pdev->dev, ha->queues_len,
 					&ha->queues_dma, GFP_KERNEL);
 	if (ha->queues == NULL) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Memory Allocation failed - queues.\n");
-
+		dev_warn(&ha->pdev->dev,
+			 "Memory Allocation failed - queues.\n");
 		goto mem_alloc_error_exit;
 	}
 	memset(ha->queues, 0, ha->queues_len);

drivers/scsi/qla4xxx/ql4_os.c static int qla4xxx_mem_alloc(struct scsi · line 830

Quoted 9 lines
drivers/scsi/qla4xxx/ql4_os.c +2 -3
@@ -836,9 +825,8 @@ static int qla4xxx_mem_alloc(struct scsi
 	ha->srb_mempool = mempool_create(SRB_MIN_REQ, mempool_alloc_slab,
 					 mempool_free_slab, srb_cachep);
 	if (ha->srb_mempool == NULL) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Memory Allocation failed - SRB Pool.\n");
-
+		dev_warn(&ha->pdev->dev,
+			 "Memory Allocation failed - SRB Pool.\n");
 		goto mem_alloc_error_exit;

Sorry for the late response on this one. As you know I was out for a while and I was waiting to get internet access yesterday.

For these host messages, do we want something like the sdev_printk and starget_printk or does it really make more sense to use the pci bus id for the message prefix? What about other scsi host messages, will they always go with the pci bus id or some scsi-ml id? And even if we want to print out the pci bus id as the prefix instead of some scsi info, should we still have some scsi wrapper?

author
#5 lore raw

Re: [PATCH ver3 3/4] qla4xxx: use dev_xxx on some pci/dma resource alloc warning and error printks

On Wed, 09 Aug 2006 13:52:07 EDT, Mike Christie wrote:
...

Quoted 11 lines

drivers/scsi/qla4xxx/ql4_os.c static int qla4xxx_mem_alloc(struct scsi · line 830

Quoted 7 lines
drivers/scsi/qla4xxx/ql4_os.c +2 -3
 	if (ha->srb_mempool == NULL) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Memory Allocation failed - SRB Pool.\n");
-
+		dev_warn(&ha->pdev->dev,
+			 "Memory Allocation failed - SRB Pool.\n");
 		goto mem_alloc_error_exit;

Sorry for the late response on this one. As you know I was out for a while and I was waiting to get internet access yesterday.

Quoted 7 lines

For these host messages, do we want something like the sdev_printk and starget_printk or does it really make more sense to use the pci bus id for the message prefix? What about other scsi host messages, will they always go with the pci bus id or some scsi-ml id? And even if we want to print out the pci bus id as the prefix instead of some scsi info, should we still have some scsi wrapper?

Ha. Just pre-replied to the this issue in the patchset just sent.

Will spin up an RFC patch and see where this goes.

author
#6 lore raw

Re: [PATCH ver3 3/4] qla4xxx: use dev_xxx on some pci/dma resource alloc warning and error printks

In reply to Mike Christie

On Wed, 09 Aug 2006 13:52:07 EDT, Mike Christie wrote:
...

Quoted 14 lines

drivers/scsi/qla4xxx/ql4_os.c static int qla4xxx_mem_alloc(struct scsi · line 830

Quoted 10 lines
drivers/scsi/qla4xxx/ql4_os.c +2 -3
@@ -836,9 +825,8 @@ static int qla4xxx_mem_alloc(struct scsi
 	ha->srb_mempool = mempool_create(SRB_MIN_REQ, mempool_alloc_slab,
 					 mempool_free_slab, srb_cachep);
 	if (ha->srb_mempool == NULL) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Memory Allocation failed - SRB Pool.\n");
-
+		dev_warn(&ha->pdev->dev,
+			 "Memory Allocation failed - SRB Pool.\n");
 		goto mem_alloc_error_exit;

Sorry for the late response on this one. As you know I was out for a while and I was waiting to get internet access yesterday.

Quoted 7 lines

For these host messages, do we want something like the sdev_printk and starget_printk or does it really make more sense to use the pci bus id for the message prefix? What about other scsi host messages, will they always go with the pci bus id or some scsi-ml id? And even if we want to print out the pci bus id as the prefix instead of some scsi info, should we still have some scsi wrapper?

I do agree that iscsi_transport sessions could use a new macro.

My intention in this instance was to go with the widely used idiom, and to not have a driver specific one. Was trying to replace

 #define ql4_printk(level, ha, format, arg...)			\
     dev_printk(level, &((ha)->pdev->dev), format, ## arg)

But to follow on from irc, one more pass at this to help me clarify and understand what is need here.

dev_xxx is a wrapper around dev_print(xxx ...).

In the specific instance above, this should print
scsiN arg...

Since qla4xxx_mem_alloc() is for the host, would an sdev_printk be the right thing? I don't believe we have any context of a target. Of course that may be a misunderstanding on my part.

For this driver, I don't see any instances of a scsi_target.

++doug

#7 lore raw

Re: [PATCH ver3 3/4] qla4xxx: use dev_xxx on some pci/dma resource alloc warning and error printks

Quoted 37 lines · Doug Maxey

On Wed, 09 Aug 2006 13:52:07 EDT, Mike Christie wrote:
...

Quoted 14 lines

drivers/scsi/qla4xxx/ql4_os.c static int qla4xxx_mem_alloc(struct scsi · line 830

Quoted 10 lines
drivers/scsi/qla4xxx/ql4_os.c +2 -3
@@ -836,9 +825,8 @@ static int qla4xxx_mem_alloc(struct scsi
 	ha->srb_mempool = mempool_create(SRB_MIN_REQ, mempool_alloc_slab,
 					 mempool_free_slab, srb_cachep);
 	if (ha->srb_mempool == NULL) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Memory Allocation failed - SRB Pool.\n");
-
+		dev_warn(&ha->pdev->dev,
+			 "Memory Allocation failed - SRB Pool.\n");
 		goto mem_alloc_error_exit;

Sorry for the late response on this one. As you know I was out for a while and I was waiting to get internet access yesterday.

Quoted 7 lines

For these host messages, do we want something like the sdev_printk and starget_printk or does it really make more sense to use the pci bus id for the message prefix? What about other scsi host messages, will they always go with the pci bus id or some scsi-ml id? And even if we want to print out the pci bus id as the prefix instead of some scsi info, should we still have some scsi wrapper?

I do agree that iscsi_transport sessions could use a new macro.

My intention in this instance was to go with the widely used idiom, and to not have a driver specific one. Was trying to replace

 #define ql4_printk(level, ha, format, arg...)			\
     dev_printk(level, &((ha)->pdev->dev), format, ## arg)

But to follow on from irc, one more pass at this to help me clarify and understand what is need here.

dev_xxx is a wrapper around dev_print(xxx ...).

Quoted 8 lines

In the specific instance above, this should print
scsiN arg...

Since qla4xxx_mem_alloc() is for the host, would an sdev_printk be the right thing? I don't believe we have any context of a target. Of course that may be a misunderstanding on my part.

For this driver, I don't see any instances of a scsi_target.

You are correct. Its during init time. So you dont have any instance of scsi_target.

Ravi

#8 lore raw

Re: [PATCH ver3 3/4] qla4xxx: use dev_xxx on some pci/dma resource alloc warning and error printks

In reply to Doug Maxey
Quoted 30 lines · Doug Maxey

On Wed, 09 Aug 2006 13:52:07 EDT, Mike Christie wrote:
...

Quoted 13 lines

drivers/scsi/qla4xxx/ql4_os.c static int qla4xxx_mem_alloc(struct scsi · line 830

Quoted 10 lines
drivers/scsi/qla4xxx/ql4_os.c +2 -3
@@ -836,9 +825,8 @@ static int qla4xxx_mem_alloc(struct scsi
 	ha->srb_mempool = mempool_create(SRB_MIN_REQ, mempool_alloc_slab,
 					 mempool_free_slab, srb_cachep);
 	if (ha->srb_mempool == NULL) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Memory Allocation failed - SRB Pool.\n");
-
+		dev_warn(&ha->pdev->dev,
+			 "Memory Allocation failed - SRB Pool.\n");
 		goto mem_alloc_error_exit;

Sorry for the late response on this one. As you know I was out for a while and I was waiting to get internet access yesterday.

Quoted 7 lines

For these host messages, do we want something like the sdev_printk and starget_printk or does it really make more sense to use the pci bus id for the message prefix? What about other scsi host messages, will they always go with the pci bus id or some scsi-ml id? And even if we want to print out the pci bus id as the prefix instead of some scsi info, should we still have some scsi wrapper?

I do agree that iscsi_transport sessions could use a new macro.

My intention in this instance was to go with the widely used idiom, and to not have a driver specific one. Was trying to replace

 #define ql4_printk(level, ha, format, arg...)			\
     dev_printk(level, &((ha)->pdev->dev), format, ## arg)
Quoted 7 lines

But to follow on from irc, one more pass at this to help me clarify and understand what is need here.

dev_xxx is a wrapper around dev_print(xxx ...).

In the specific instance above, this should print
scsiN arg...

I think the ones in qla4xxx_mem_alloc print

%pci_id: arg...

Quoted 3 lines

Since qla4xxx_mem_alloc() is for the host, would an sdev_printk be the right thing? I don't believe we have any context of a target.

I do not think so. To answer the sdev_printk question for future reference and maybe to clear things up, I think sdev_printk is for struct scsi_devices so it should be used for messages about a scsi device. I think where we may be missing each other on this is that sdev is a common abbreviation for the scsi_device (as in scsi SPEC logic unit). It is not a abbreviation for scsi device as in any old scsi object that has a struct device in it or is a device that scsi messes with. So maybe in functions like the device reset eh callback we could use sdev_printk. For functions just interacting with the scsi host that want to print something about the host you would not.

But to really answer what we should have been taking about, I think just using dev_printk is probably ok for now. When you said in some other mail, that you thought we needed to use dev_printk, I thought you meant that we needed to covert all the messages to dev_printk or one of the scsi wrappers around it. After reading the mail above that you are just removing the needless ql4_printk wrapper, I think just replacing ql4_printk usage with dev_printk is ok for now.

Quoted 4 lines

Of course that may be a misunderstanding on my part.

For this driver, I don't see any instances of a scsi_target.

The driver does not really deal with struct scsi_targets. The driver interacts with structs like iscsi_sessions or iscsi_connections. The iscsi_session hides most of the struct scsi_target details from the driver. My mention of the scsi target was just because there is a starget_printk and I was wondering if we need a shost_printk that the LLD could use and that would complete the API.

For the iscsi conversion comment on irc, in scsi_transport_iscsi.c, we do dev_printk(&session->dev and dev_printk(&connection->dev, and we could maybe use some wrapper around them (like a iscsi_conn_printk() and iscsi_sess_printk() that take a session and connection) like is done for scsi_target and scsi_device structs and we could then use that in libiscsi and iscsi_tcp and iscsi_iser. Right now in some of the iscsi code we have some silly error messages that just spit out a error but do not tell you what session, connection or host or device it is for.

#9 lore raw

Re: [PATCH ver3 3/4] qla4xxx: use dev_xxx on some pci/dma resource alloc warning and error printks

Quoted 34 lines · Mike Christie
Quoted 33 lines · Doug Maxey

On Wed, 09 Aug 2006 13:52:07 EDT, Mike Christie wrote:
...

Quoted 13 lines

drivers/scsi/qla4xxx/ql4_os.c static int qla4xxx_mem_alloc(struct scsi · line 830

Quoted 10 lines
drivers/scsi/qla4xxx/ql4_os.c +2 -3
@@ -836,9 +825,8 @@ static int qla4xxx_mem_alloc(struct scsi
 	ha->srb_mempool = mempool_create(SRB_MIN_REQ, mempool_alloc_slab,
 					 mempool_free_slab, srb_cachep);
 	if (ha->srb_mempool == NULL) {
-		ql4_printk(KERN_WARNING, ha,
-			   "Memory Allocation failed - SRB Pool.\n");
-
+		dev_warn(&ha->pdev->dev,
+			 "Memory Allocation failed - SRB Pool.\n");
 		goto mem_alloc_error_exit;

Sorry for the late response on this one. As you know I was out for a while and I was waiting to get internet access yesterday.

Quoted 7 lines

For these host messages, do we want something like the sdev_printk and starget_printk or does it really make more sense to use the pci bus id for the message prefix? What about other scsi host messages, will they always go with the pci bus id or some scsi-ml id? And even if we want to print out the pci bus id as the prefix instead of some scsi info, should we still have some scsi wrapper?

I do agree that iscsi_transport sessions could use a new macro.

My intention in this instance was to go with the widely used idiom, and to not have a driver specific one. Was trying to replace

 #define ql4_printk(level, ha, format, arg...)			\
     dev_printk(level, &((ha)->pdev->dev), format, ## arg)

But to follow on from irc, one more pass at this to help me clarify and understand what is need here.

Quoted 9 lines
Quoted 4 lines

dev_xxx is a wrapper around dev_print(xxx ...).

In the specific instance above, this should print
scsiN arg...

I think the ones in qla4xxx_mem_alloc print

%pci_id: arg...

Maybe I was wrong...

Given that some messages are printing the pci bus id (the ones like above in qla4xxx_mem_alloc using ql4_printk), and other messages are printing the host number (the ones using printk with "scsi%ld" coded in them) maybe this means....

Quoted 3 lines

removing the needless ql4_printk wrapper, I think just replacing ql4_printk usage with dev_printk is ok for now.

that this comment is wrong and maybe we need to to more.

Quoted 3 lines

starget_printk and I was wondering if we need a shost_printk that the LLD could use and that would complete the API.

And maybe we need a shost_printk to go along with the sdev and starget printks, so drivers do not end up printing different prefixes for the same objects like is happening with qla4xxx.

#10 lore raw

Re: [PATCH ver3 3/4] qla4xxx: use dev_xxx on some pci/dma resource alloc warning and error printks

Quoted 3 lines · Mike Christie

And maybe we need a shost_printk to go along with the sdev and starget printks, so drivers do not end up printing different prefixes for the same objects like is happening with qla4xxx.

Sure ... add it if you need it.

James

author
#11 lore raw

PATCH 4/4 qla4xxx: improve symmetry in buffer codepaths

In reply to a message not in the archive

From: Doug Maxey <[email protected]>

In qla4xxx_send_command_to_isp(), pci_map_*() is used. At the other end, in qla4xxx_srb_free_dma(), dma_unmap_*() is used. Replace the call with the symmetrical call to pci_unmap_*().

Signed-off-by Doug Maxey <[email protected]>

---

 drivers/scsi/qla4xxx/ql4_os.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/qla4xxx/ql4_os.c +2 -2
index d88c84f..5e10f30 100644
@@ -382,10 +382,10 @@ static void qla4xxx_srb_free_dma(struct 
 
 	if (srb->flags & SRB_DMA_VALID) {
 		if (cmd->use_sg) {
-			dma_unmap_sg(&ha->pdev->dev, cmd->request_buffer,
+			pci_unmap_sg(ha->pdev, cmd->request_buffer,
 				     cmd->use_sg, cmd->sc_data_direction);
 		} else if (cmd->request_bufflen) {
-			dma_unmap_single(&ha->pdev->dev, srb->dma_handle,
+			pci_unmap_single(ha->pdev, srb->dma_handle,
 					 cmd->request_bufflen,
 					 cmd->sc_data_direction);
 		}