All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1.0 7/16] arcmsr: revise message_isr_bh_fn to delete duplicate code
@ 2014-04-30 11:02 ching
  2014-05-02  8:49 ` Dan Carpenter
  0 siblings, 1 reply; 2+ messages in thread
From: ching @ 2014-04-30 11:02 UTC (permalink / raw)
  To: jbottomley, dan.carpenter, thenzl, linux-scsi, linux-kernel

From: Ching<ching2048@areca.com.tw>

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

Signed-off-by: Ching<ching2048@areca.com.tw>
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c	2014-04-30 15:10:02.000000000 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c	2014-04-30 15:10:46.000000000 +0800
@@ -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++;
 		}
 	}
 }



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v1.0 7/16] arcmsr: revise message_isr_bh_fn to delete duplicate code
  2014-04-30 11:02 [PATCH v1.0 7/16] arcmsr: revise message_isr_bh_fn to delete duplicate code ching
@ 2014-05-02  8:49 ` Dan Carpenter
  0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2014-05-02  8:49 UTC (permalink / raw)
  To: ching; +Cc: jbottomley, thenzl, linux-scsi, linux-kernel

On Wed, Apr 30, 2014 at 07:02:37PM +0800, ching wrote:
> +	}
> +	atomic_inc(&acb->rq_map_token);

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

> +	if (readl(signature) == ARCMSR_SIGNATURE_GET_CONFIG) {

	if (readl(signature) != ARCMSR_SIGNATURE_GET_CONFIG)
		return;

> +		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);

The whitespace is weird here.  Do:

					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++;
>  		}

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-05-02  8:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-30 11:02 [PATCH v1.0 7/16] arcmsr: revise message_isr_bh_fn to delete duplicate code ching
2014-05-02  8:49 ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.