Skip to contentSkip to conversation

arcmsr: revise message_isr_bh_fn to delete duplicate code

PATCH In review ching 2 participants 2 messages Apr 30, 2014 – May 2, 2014 SCSI

lore
Share & tools
lore

ching

author
#0 lore raw

PATCH 7/16 arcmsr: revise message_isr_bh_fn to delete duplicate code

From: Ching<[email protected]>

Revise message_isr_bh_fn to remove the duplicate code for each adapter type.

---

drivers/scsi/arcmsr/arcmsr_hba.c +51 -105
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -511,119 +511,65 @@ static int arcmsr_alloc_ccb_pool(struct 
 
 static void arcmsr_message_isr_bh_fn(struct work_struct *work) 
 {
-	struct AdapterControlBlock *acb = container_of(work,struct AdapterControlBlock, arcmsr_do_message_isr_bh);
-	switch (acb->adapter_type) {
-		case ACB_ADAPTER_TYPE_A: {
+	struct AdapterControlBlock *acb = container_of(work,
+		struct AdapterControlBlock, arcmsr_do_message_isr_bh);
+	char *acb_dev_map = (char *)acb->device_map;
+	uint32_t __iomem *signature = NULL;
+	char __iomem *devicemap = NULL;
+	int target, lun;
+	struct scsi_device *psdev;
+	char diff, temp;
 
-			struct MessageUnit_A __iomem *reg  = acb->pmuA;
-			char *acb_dev_map = (char *)acb->device_map;
-			uint32_t __iomem *signature = (uint32_t __iomem*) (&reg->message_rwbuffer[0]);
-			char __iomem *devicemap = (char __iomem*) (&reg->message_rwbuffer[21]);
-			int target, lun;
-			struct scsi_device *psdev;
-			char diff;
-
-			atomic_inc(&acb->rq_map_token);
-			if (readl(signature) == ARCMSR_SIGNATURE_GET_CONFIG) {
-				for(target = 0; target < ARCMSR_MAX_TARGETID -1; target++) {
-					diff = (*acb_dev_map)^readb(devicemap);
-					if (diff != 0) {
-						char temp;
-						*acb_dev_map = readb(devicemap);
-						temp =*acb_dev_map;
-						for(lun = 0; lun < ARCMSR_MAX_TARGETLUN; lun++) {
-							if((temp & 0x01)==1 && (diff & 0x01) == 1) {	
-								scsi_add_device(acb->host, 0, target, lun);
-							}else if((temp & 0x01) == 0 && (diff & 0x01) == 1) {
-								psdev = scsi_device_lookup(acb->host, 0, target, lun);
-								if (psdev != NULL ) {
-									scsi_remove_device(psdev);
-									scsi_device_put(psdev);
-								}
-							}
-							temp >>= 1;
-							diff >>= 1;
-						}
-					}
-					devicemap++;
-					acb_dev_map++;
-				}
-			}
-			break;
+	switch (acb->adapter_type) {
+	case ACB_ADAPTER_TYPE_A: {
+		struct MessageUnit_A __iomem *reg  = acb->pmuA;
+		signature = (uint32_t __iomem *)(&reg->message_rwbuffer[0]);
+		devicemap = (char __iomem *)(&reg->message_rwbuffer[21]);
+		break;
 		}
-
-		case ACB_ADAPTER_TYPE_B: {
-			struct MessageUnit_B *reg  = acb->pmuB;
-			char *acb_dev_map = (char *)acb->device_map;
-			uint32_t __iomem *signature = (uint32_t __iomem*)(&reg->message_rwbuffer[0]);
-			char __iomem *devicemap = (char __iomem*)(&reg->message_rwbuffer[21]);
-			int target, lun;
-			struct scsi_device *psdev;
-			char diff;
-
-			atomic_inc(&acb->rq_map_token);
-			if (readl(signature) == ARCMSR_SIGNATURE_GET_CONFIG) {
-				for(target = 0; target < ARCMSR_MAX_TARGETID -1; target++) {
-					diff = (*acb_dev_map)^readb(devicemap);
-					if (diff != 0) {
-						char temp;
-						*acb_dev_map = readb(devicemap);
-						temp =*acb_dev_map;
-						for(lun = 0; lun < ARCMSR_MAX_TARGETLUN; lun++) {
-							if((temp & 0x01)==1 && (diff & 0x01) == 1) {	
-								scsi_add_device(acb->host, 0, target, lun);
-							}else if((temp & 0x01) == 0 && (diff & 0x01) == 1) {
-								psdev = scsi_device_lookup(acb->host, 0, target, lun);
-								if (psdev != NULL ) {
-									scsi_remove_device(psdev);
-									scsi_device_put(psdev);
-								}
-							}
-							temp >>= 1;
-							diff >>= 1;
-						}
-					}
-					devicemap++;
-					acb_dev_map++;
-				}
-			}
+	case ACB_ADAPTER_TYPE_B: {
+		struct MessageUnit_B *reg  = acb->pmuB;
+		signature = (uint32_t __iomem *)(&reg->message_rwbuffer[0]);
+		devicemap = (char __iomem *)(&reg->message_rwbuffer[21]);
+		break;
 		}
+	case ACB_ADAPTER_TYPE_C: {
+		struct MessageUnit_C __iomem *reg  = acb->pmuC;
+		signature = (uint32_t __iomem *)(&reg->msgcode_rwbuffer[0]);
+		devicemap = (char __iomem *)(&reg->msgcode_rwbuffer[21]);
 		break;
-		case ACB_ADAPTER_TYPE_C: {
-			struct MessageUnit_C *reg  = acb->pmuC;
-			char *acb_dev_map = (char *)acb->device_map;
-			uint32_t __iomem *signature = (uint32_t __iomem *)(&reg->msgcode_rwbuffer[0]);
-			char __iomem *devicemap = (char __iomem *)(&reg->msgcode_rwbuffer[21]);
-			int target, lun;
-			struct scsi_device *psdev;
-			char diff;
-
-			atomic_inc(&acb->rq_map_token);
-			if (readl(signature) == ARCMSR_SIGNATURE_GET_CONFIG) {
-				for (target = 0; target < ARCMSR_MAX_TARGETID - 1; target++) {
-					diff = (*acb_dev_map)^readb(devicemap);
-					if (diff != 0) {
-						char temp;
-						*acb_dev_map = readb(devicemap);
-						temp = *acb_dev_map;
-						for (lun = 0; lun < ARCMSR_MAX_TARGETLUN; lun++) {
-							if ((temp & 0x01) == 1 && (diff & 0x01) == 1) {
-								scsi_add_device(acb->host, 0, target, lun);
-							} else if ((temp & 0x01) == 0 && (diff & 0x01) == 1) {
-								psdev = scsi_device_lookup(acb->host, 0, target, lun);
-								if (psdev != NULL) {
-									scsi_remove_device(psdev);
-									scsi_device_put(psdev);
-								}
-							}
-							temp >>= 1;
-							diff >>= 1;
+		}
+	}
+	atomic_inc(&acb->rq_map_token);
+	if (readl(signature) == ARCMSR_SIGNATURE_GET_CONFIG) {
+		for (target = 0; target < ARCMSR_MAX_TARGETID - 1;
+			target++) {
+			temp = readb(devicemap);
+			diff = (*acb_dev_map) ^ temp;
+			if (diff != 0) {
+				*acb_dev_map = temp;
+				for (lun = 0; lun < ARCMSR_MAX_TARGETLUN;
+					lun++) {
+					if ((diff & 0x01) == 1 &&
+						(temp & 0x01) == 1) {
+						scsi_add_device(acb->host,
+							0, target, lun);
+					} else if ((diff & 0x01) == 1
+						&& (temp & 0x01) == 0) {
+						psdev =
+							scsi_device_lookup(acb->host,
+							0, target, lun);
+						if (psdev != NULL) {
+							scsi_remove_device(psdev);
+							scsi_device_put(psdev);
 						}
 					}
-					devicemap++;
-					acb_dev_map++;
+					temp >>= 1;
+					diff >>= 1;
 				}
 			}
+			devicemap++;
+			acb_dev_map++;
 		}
 	}
 }
#1 lore raw

Review

Quoted 2 lines · ching
(quoted diff) +2 -0
+	}
+	atomic_inc(&acb->rq_map_token);

This patch is a nice cleanup but it would be even better if you flipped this condition around.

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_alloc_ccb_pool(struct · line 544

Quoted 1 line
drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+	if (readl(signature) == ARCMSR_SIGNATURE_GET_CONFIG) {
	if (readl(signature) != ARCMSR_SIGNATURE_GET_CONFIG)
		return;
Quoted 7 lines
drivers/scsi/arcmsr/arcmsr_hba.c +7 -0
+		for (target = 0; target < ARCMSR_MAX_TARGETID - 1;
+			target++) {
+			temp = readb(devicemap);
+			diff = (*acb_dev_map) ^ temp;
+			if (diff != 0) {
+				*acb_dev_map = temp;
+				for (lun = 0; lun < ARCMSR_MAX_TARGETLUN;

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_alloc_ccb_pool(struct · line 561

Quoted 10 lines
drivers/scsi/arcmsr/arcmsr_hba.c +10 -0
+					lun++) {
+					if ((diff & 0x01) == 1 &&
+						(temp & 0x01) == 1) {
+						scsi_add_device(acb->host,
+							0, target, lun);
+					} else if ((diff & 0x01) == 1
+						&& (temp & 0x01) == 0) {
+						psdev =
+							scsi_device_lookup(acb->host,
+							0, target, lun);

The whitespace is weird here. Do:

					psdev = scsi_device_lookup(acb->host,
							0, target, lun);
Quoted 4 lines
drivers/scsi/arcmsr/arcmsr_hba.c +3 -0
+						if (psdev != NULL) {
+							scsi_remove_device(psdev);
+							scsi_device_put(psdev);
 						}

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_alloc_ccb_pool(struct · line 573

Quoted 10 lines
drivers/scsi/arcmsr/arcmsr_hba.c +4 -2
 					}
-					devicemap++;
-					acb_dev_map++;
+					temp >>= 1;
+					diff >>= 1;
 				}
 			}
+			devicemap++;
+			acb_dev_map++;
 		}

regards,
dan carpenter