arcmsr: code cleanup and some corrections

PATCH In review Tomas Henzl 3 participants 15 messages Feb 10, 2011 – Feb 25, 2011 SCSI

Share & tools

Tomas Henzl

author

2011-02-10 14:22 UTC #0 lore raw

PATCH 1/2 arcmsr: code cleanup and some corrections

I removed outer loops in ...wait_msgint_ready
the sleeptime and retrycount are in fact never changed so I changed them into defines. In arcmsr_flush_hba_cache is a loop removed, which printed the same printk 100 times, one line in log is enough I think. The arcmsr_sleep_for_bus_reset has lost a functionality with the latest patches, The only thing the function does is a long sleep, so it's replaced with a ssleep.

Signed-off-by Tomas henzl <[email protected]>

drivers/scsi/arcmsr/arcmsr_hba.c +48 ~~-70~~

index 984bd52..4cd522b 100644
@@ -75,8 +75,10 @@ MODULE_AUTHOR("Nick Cheng <[email protected]>");
 MODULE_DESCRIPTION("ARECA (ARC11xx/12xx/16xx/1880) SATA/SAS RAID Host Bus Adapter");
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(ARCMSR_DRIVER_VERSION);
-static int sleeptime = 10;
-static int retrycount = 12;
+
+#define	ARCMSR_SLEEPTIME	10
+#define	ARCMSR_RETRYCOUNT	12
+
 wait_queue_head_t wait_q;
 static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
 					struct scsi_cmnd *cmd);

@@ -171,24 +173,6 @@ static struct pci_driver arcmsr_pci_driver = {
 ****************************************************************************
 ****************************************************************************
 */
-int arcmsr_sleep_for_bus_reset(struct scsi_cmnd *cmd)
-{
-		struct Scsi_Host *shost = NULL;
-		int i, isleep;
-		shost = cmd->device->host;
-		isleep = sleeptime / 10;
-		if (isleep > 0) {
-			for (i = 0; i < isleep; i++) {
-				msleep(10000);
-			}
-		}
-
-		isleep = sleeptime % 10;
-		if (isleep > 0) {
-			msleep(isleep*1000);
-		}
-		return 0;
-}
 
 static void arcmsr_free_hbb_mu(struct AdapterControlBlock *acb)
 {

@@ -323,66 +307,64 @@ static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
 
 	default: acb->adapter_type = ACB_ADAPTER_TYPE_A;
 	}
-}	
+}
 
 static uint8_t arcmsr_hba_wait_msgint_ready(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&reg->outbound_intstatus) &
-					ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
-				writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
-					&reg->outbound_intstatus);
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&reg->outbound_intstatus) &
+				ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
+			writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
+				&reg->outbound_intstatus);
+			return true;
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbb_wait_msgint_ready(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_B *reg = acb->pmuB;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(reg->iop2drv_doorbell)
-				& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
-				writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN
-					, reg->iop2drv_doorbell);
-				writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT, reg->drv2iop_doorbell);
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(reg->iop2drv_doorbell)
+			& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
+			writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN,
+					reg->iop2drv_doorbell);
+			writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
+					reg->drv2iop_doorbell);
+			return true;
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbc_wait_msgint_ready(struct AdapterControlBlock *pACB)
 {
 	struct MessageUnit_C *phbcmu = (struct MessageUnit_C *)pACB->pmuC;
-	unsigned char Retries = 0x00;
-	uint32_t Index;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&phbcmu->outbound_doorbell) & ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
-				writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR, &phbcmu->outbound_doorbell_clear);/*clear interrupt*/
-				return true;
-			}
-			/* one us delay	*/
-			msleep(10);
-		} /*max 1 seconds*/
-	} while (Retries++ < 20); /*max 20 sec*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&phbcmu->outbound_doorbell)
+				& ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
+			writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR,
+				&phbcmu->outbound_doorbell_clear); /*clear interrupt*/
+			return true;
+		}
+		msleep(10);
+	} /* max 20 seconds */
+
 	return false;
 }
+
 static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;

@@ -2602,12 +2584,8 @@ static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
 		if (cdb_phyaddr_hi32 != 0) {
 			struct MessageUnit_C *reg = (struct MessageUnit_C *)acb->pmuC;
 
-			if (cdb_phyaddr_hi32 != 0) {
-				unsigned char Retries = 0x00;
-				do {
-					printk(KERN_NOTICE "arcmsr%d: cdb_phyaddr_hi32=0x%x \n", acb->adapter_index, cdb_phyaddr_hi32);
-				} while (Retries++ < 100);
-			}
+			printk(KERN_NOTICE "arcmsr%d: cdb_phyaddr_hi32=0x%x \n",
+					acb->adapter_index, cdb_phyaddr_hi32);
 			writel(ARCMSR_SIGNATURE_SET_CONFIG, &reg->msgcode_rwbuffer[0]);
 			writel(cdb_phyaddr_hi32, &reg->msgcode_rwbuffer[1]);
 			writel(ARCMSR_INBOUND_MESG0_SET_CONFIG, &reg->inbound_msgaddr0);

@@ -2955,10 +2933,10 @@ static int arcmsr_bus_reset(struct scsi_cmnd *cmd)
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep_again:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->outbound_msgaddr1) & ARCMSR_OUTBOUND_MESG1_FIRMWARE_OK) == 0) {
 					printk(KERN_ERR "arcmsr%d: waiting for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT) {
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d: waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
 						return FAILED;

@@ -3025,10 +3003,10 @@ sleep_again:
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->host_diagnostic) & 0x04) != 0) {
 					printk(KERN_ERR "arcmsr%d: waiting for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT) {
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d: waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
 						return FAILED;

Tomas Henzl

author

2011-02-10 14:36 UTC #1 lore raw

PATCH 2/2 arcmsr: code cleanup and some corrections

Hi Nick,
I'm confused with what this code does, I think I must miss something.

	dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size, &dma_coherent_handle, GFP_KERNEL);

...
I think that the dma_alloc_coherent returns a page address with zeroes in the lowest bits That would mean the offset computed below is useless as the result is zero every time. Added to that is that you then increase dma_coherent_handle by the offset,
while dma_coherent is increased by multiples of sizeof CommandControlBlock, at least it looks so
...

	offset = roundup((unsigned long)dma_coherent, 32) - (unsigned long)dma_coherent;
	dma_coherent_handle = dma_coherent_handle + offset;
	dma_coherent = (struct CommandControlBlock *)dma_coherent + offset;

The patch below removes the offset computation.

Signed-off-by Tomas henzl <[email protected]>

drivers/scsi/arcmsr/arcmsr_hba.c +4 -6

index 4cd522b..da93974 100644
@@ -441,10 +441,11 @@ static int arcmsr_alloc_ccb_pool(struct AdapterControlBlock *acb)
 	struct CommandControlBlock *ccb_tmp;
 	int i = 0, j = 0;
 	dma_addr_t cdb_phyaddr;
-	unsigned long roundup_ccbsize = 0, offset;
+	unsigned long roundup_ccbsize;
 	unsigned long max_xfer_len;
 	unsigned long max_sg_entrys;
 	uint32_t  firm_config_version;
+
 	for (i = 0; i < ARCMSR_MAX_TARGETID; i++)
 		for (j = 0; j < ARCMSR_MAX_TARGETLUN; j++)
 			acb->devstate[i][j] = ARECA_RAID_GONE;

@@ -454,12 +455,12 @@ static int arcmsr_alloc_ccb_pool(struct AdapterControlBlock *acb)
 	firm_config_version = acb->firm_cfg_version;
 	if((firm_config_version & 0xFF) >= 3){
 		max_xfer_len = (ARCMSR_CDB_SG_PAGE_LENGTH << ((firm_config_version >> 8) & 0xFF)) * 1024;/* max 4M byte */
-		max_sg_entrys = (max_xfer_len/4096);	
+		max_sg_entrys = (max_xfer_len/4096);
 	}
 	acb->host->max_sectors = max_xfer_len/512;
 	acb->host->sg_tablesize = max_sg_entrys;
 	roundup_ccbsize = roundup(sizeof(struct CommandControlBlock) + (max_sg_entrys - 1) * sizeof(struct SG64ENTRY), 32);
-	acb->uncache_size = roundup_ccbsize * ARCMSR_MAX_FREECCB_NUM + 32;
+	acb->uncache_size = roundup_ccbsize * ARCMSR_MAX_FREECCB_NUM;
 	dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size, &dma_coherent_handle, GFP_KERNEL);
 	if(!dma_coherent){
 		printk(KERN_NOTICE "arcmsr%d: dma_alloc_coherent got error \n", acb->host->host_no);

@@ -468,9 +469,6 @@ static int arcmsr_alloc_ccb_pool(struct AdapterControlBlock *acb)
 	acb->dma_coherent = dma_coherent;
 	acb->dma_coherent_handle = dma_coherent_handle;
 	memset(dma_coherent, 0, acb->uncache_size);
-	offset = roundup((unsigned long)dma_coherent, 32) - (unsigned long)dma_coherent;
-	dma_coherent_handle = dma_coherent_handle + offset;
-	dma_coherent = (struct CommandControlBlock *)dma_coherent + offset;
 	ccb_tmp = dma_coherent;
 	acb->vir2phy_offset = (unsigned long)dma_coherent - (unsigned long)dma_coherent_handle;
 	for(i = 0; i < ARCMSR_MAX_FREECCB_NUM; i++){

NickCheng

2011-02-11 01:45 UTC #2 lore raw

Hi Tomas,
As you said, the dma_alloc_coherent should return a page address with zeroes in the lowest bits, but I have 100% confidence in that. Therefore, I try to round it up if not. Otherwise, it will lead the controller panic.
The code over there is just in case.

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Thursday, February 10, 2011 10:37 PM
To: '[email protected]'
Cc: [email protected]
Subject: [PATCH 2/2] arcmsr: code cleanup and some corrections

Hi Nick,
I'm confused with what this code does, I think I must miss something.

	dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size,

&dma_coherent_handle, GFP_KERNEL);
...
I think that the dma_alloc_coherent returns a page address with zeroes in the lowest bits
That would mean the offset computed below is useless as the result is zero every time.
Added to that is that you then increase dma_coherent_handle by the offset, while dma_coherent is increased by multiples of sizeof CommandControlBlock, at least it looks so
...
offset = roundup((unsigned long)dma_coherent, 32) - (unsigned

long)dma_coherent;
	dma_coherent_handle = dma_coherent_handle + offset;
	dma_coherent = (struct CommandControlBlock *)dma_coherent + offset;

The patch below removes the offset computation.

Signed-off-by Tomas henzl <[email protected]>

(quoted diff) +0 -0

b/drivers/scsi/arcmsr/arcmsr_hba.c
index 4cd522b..da93974 100644

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0

@@ -441,10 +441,11 @@ static int arcmsr_alloc_ccb_pool(struct

AdapterControlBlock *acb)
struct CommandControlBlock *ccb_tmp;
int i = 0, j = 0;
dma_addr_t cdb_phyaddr;
- unsigned long roundup_ccbsize = 0, offset;
+ unsigned long roundup_ccbsize;
unsigned long max_xfer_len;
unsigned long max_sg_entrys;
uint32_t firm_config_version;
+
for (i = 0; i < ARCMSR_MAX_TARGETID; i++)
for (j = 0; j < ARCMSR_MAX_TARGETLUN; j++)
acb->devstate[i][j] = ARECA_RAID_GONE;
@@ -454,12 +455,12 @@ static int arcmsr_alloc_ccb_pool(struct AdapterControlBlock *acb)
firm_config_version = acb->firm_cfg_version; if((firm_config_version & 0xFF) >= 3){
max_xfer_len = (ARCMSR_CDB_SG_PAGE_LENGTH <<
((firm_config_version >> 8) & 0xFF)) * 1024;/* max 4M byte */
- max_sg_entrys = (max_xfer_len/4096);
+ max_sg_entrys = (max_xfer_len/4096);
}
acb->host->max_sectors = max_xfer_len/512; acb->host->sg_tablesize = max_sg_entrys; roundup_ccbsize = roundup(sizeof(struct CommandControlBlock) +
(max_sg_entrys - 1) * sizeof(struct SG64ENTRY), 32);
- acb->uncache_size = roundup_ccbsize * ARCMSR_MAX_FREECCB_NUM + 32;
+ acb->uncache_size = roundup_ccbsize * ARCMSR_MAX_FREECCB_NUM;
dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size,
&dma_coherent_handle, GFP_KERNEL);
if(!dma_coherent){
printk(KERN_NOTICE "arcmsr%d: dma_alloc_coherent got error
\n", acb->host->host_no);
@@ -468,9 +469,6 @@ static int arcmsr_alloc_ccb_pool(struct AdapterControlBlock *acb)
acb->dma_coherent = dma_coherent; acb->dma_coherent_handle = dma_coherent_handle; memset(dma_coherent, 0, acb->uncache_size);
- offset = roundup((unsigned long)dma_coherent, 32) - (unsigned
long)dma_coherent;
- dma_coherent_handle = dma_coherent_handle + offset;
- dma_coherent = (struct CommandControlBlock *)dma_coherent + offset;
ccb_tmp = dma_coherent;
acb->vir2phy_offset = (unsigned long)dma_coherent - (unsigned
long)dma_coherent_handle;
for(i = 0; i < ARCMSR_MAX_FREECCB_NUM; i++){

Tomas Henzl

author

2011-02-17 15:05 UTC #3 lore raw

James,

the discussion is mostly if 'dma_alloc_coherent'
...
dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size, &dma_coherent_handle, GFP_KERNEL);
...
returns a page address - dma_coherent with last, I think five bits, zeroed.
Could you help us with that?

Quoted 7 lines

Hi Tomas,
As you said, the dma_alloc_coherent should return a page address with zeroes in the lowest bits, but I have 100% confidence in that. Therefore, I try to round it up if not. Otherwise, it will lead the controller panic.
The code over there is just in case.

Nick, let me explain why I think the fix you put there 'just in case' is broken.

* if the values were set for example so : dma_coherent = 0xc1000001 dma_coherent_handle = 0xc0000001

	offset = roundup((unsigned long)dma_coherent, 32) - (unsigned long)dma_coherent;

you will get an offset = 31

	dma_coherent_handle = dma_coherent_handle + offset;

here dma_coherent_handle has the right value of dma_coherent_handle = 0xc0000020

	dma_coherent = (struct CommandControlBlock *)dma_coherent + offset;

but dma_coherent = 0xc1000b25
this seems to be wrong, the right value should be 0xc1000020, am I right?

* you will maybe get a controller panic if you pass to your card a wrong value with last bits not zeroed out, but with that computation you only ensure that dma_coherent is rounded up and not the dma_coherent_handle which you then further pass to the card.

---------
So my conclusion is that because nobody has complained, (and this has had a lot of testing) that a situation with a non null offset hasn't happened yet.

--tomash

Quoted 83 lines

Hi Nick,
I'm confused with what this code does, I think I must miss something.
	dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size,
&dma_coherent_handle, GFP_KERNEL);
...
I think that the dma_alloc_coherent returns a page address with zeroes in the lowest bits
That would mean the offset computed below is useless as the result is zero every time.
Added to that is that you then increase dma_coherent_handle by the offset, while dma_coherent is increased by multiples of sizeof CommandControlBlock, at least it looks so
...
offset = roundup((unsigned long)dma_coherent, 32) - (unsigned
long)dma_coherent;
	dma_coherent_handle = dma_coherent_handle + offset;
	dma_coherent = (struct CommandControlBlock *)dma_coherent + offset;
The patch below removes the offset computation.
Signed-off-by Tomas henzl <[email protected]>
drivers/scsi/arcmsr/arcmsr_hba.c +2 -1
index 4cd522b..da93974 100644
@@ -441,10 +441,11 @@ static int arcmsr_alloc_ccb_pool(struct
 AdapterControlBlock *acb)
 	struct CommandControlBlock *ccb_tmp;
 	int i = 0, j = 0;
 	dma_addr_t cdb_phyaddr;
-	unsigned long roundup_ccbsize = 0, offset;
+	unsigned long roundup_ccbsize;
 	unsigned long max_xfer_len;
 	unsigned long max_sg_entrys;
 	uint32_t  firm_config_version;
+
 	for (i = 0; i < ARCMSR_MAX_TARGETID; i++)
 		for (j = 0; j < ARCMSR_MAX_TARGETLUN; j++)
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 			acb->devstate[i][j] = ARECA_RAID_GONE;
@@ -454,12 +455,12 @@ static int arcmsr_alloc_ccb_pool(struct
 AdapterControlBlock *acb)
 	firm_config_version = acb->firm_cfg_version;
 	if((firm_config_version & 0xFF) >= 3){
 		max_xfer_len = (ARCMSR_CDB_SG_PAGE_LENGTH <<
((firm_config_version >> 8) & 0xFF)) * 1024;/* max 4M byte */
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
-		max_sg_entrys = (max_xfer_len/4096);	
+		max_sg_entrys = (max_xfer_len/4096);
 	}
 	acb->host->max_sectors = max_xfer_len/512;
 	acb->host->sg_tablesize = max_sg_entrys;
 	roundup_ccbsize = roundup(sizeof(struct CommandControlBlock) +
(max_sg_entrys - 1) * sizeof(struct SG64ENTRY), 32);
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
-	acb->uncache_size = roundup_ccbsize * ARCMSR_MAX_FREECCB_NUM + 32;
+	acb->uncache_size = roundup_ccbsize * ARCMSR_MAX_FREECCB_NUM;
 	dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size,
&dma_coherent_handle, GFP_KERNEL);
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 	if(!dma_coherent){
 		printk(KERN_NOTICE "arcmsr%d: dma_alloc_coherent got error
\n", acb->host->host_no);
drivers/scsi/arcmsr/arcmsr_hba.c +0 -3
@@ -468,9 +469,6 @@ static int arcmsr_alloc_ccb_pool(struct
 AdapterControlBlock *acb)
 	acb->dma_coherent = dma_coherent;
 	acb->dma_coherent_handle = dma_coherent_handle;
 	memset(dma_coherent, 0, acb->uncache_size);
-	offset = roundup((unsigned long)dma_coherent, 32) - (unsigned
 long)dma_coherent;
-	dma_coherent_handle = dma_coherent_handle + offset;
-	dma_coherent = (struct CommandControlBlock *)dma_coherent + offset;
 	ccb_tmp = dma_coherent;
acb->vir2phy_offset = (unsigned long)dma_coherent - (unsigned
long)dma_coherent_handle;
for(i = 0; i < ARCMSR_MAX_FREECCB_NUM; i++){
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

FUJITA Tomonori

2011-02-18 01:04 UTC #4 lore raw

Quoted 6 lines · On Thu, 17 Feb 2011 16:05:37 +0100 Tomas Henzl <[email protected]> wrote:

the discussion is mostly if 'dma_alloc_coherent'
...
dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size, &dma_coherent_handle, GFP_KERNEL);
...
returns a page address - dma_coherent with last, I think five bits, zeroed.
Could you help us with that?

Documentation/DMA-API-HOWTO.txt is not clear enough?

=
dma_alloc_coherent returns two values: the virtual address which you can use to access it from the CPU and dma_handle which you pass to the card.

The cpu return address and the DMA bus master address are both guaranteed to be aligned to the smallest PAGE_SIZE order which is greater than or equal to the requested size. This invariant exists (for example) to guarantee that if you allocate a chunk which is smaller than or equal to 64 kilobytes, the extent of the buffer you receive will not cross a 64K boundary.

Tomas Henzl

author

2011-02-18 11:00 UTC #5 lore raw

Quoted 18 lines · FUJITA Tomonori

Quoted 7 lines · On Thu, 17 Feb 2011 16:05:37 +0100 Tomas Henzl <[email protected]> wrote:
the discussion is mostly if 'dma_alloc_coherent'
...
dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size, &dma_coherent_handle, GFP_KERNEL);
...
returns a page address - dma_coherent with last, I think five bits, zeroed.
Could you help us with that?
Documentation/DMA-API-HOWTO.txt is not clear enough?
=
dma_alloc_coherent returns two values: the virtual address which you can use to access it from the CPU and dma_handle which you pass to the card.

Quoted 11 lines

The cpu return address and the DMA bus master address are both guaranteed to be aligned to the smallest PAGE_SIZE order which is greater than or equal to the requested size. This invariant exists (for example) to guarantee that if you allocate a chunk which is smaller than or equal to 64 kilobytes, the extent of the buffer you receive will not cross a 64K boundary.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

It's clear enough I think, I asked this question because Nick doesn't have a "100% confidence in that.". So I wanted a help to convince him.

Tomas Henzl

author

2011-02-24 14:15 UTC #6 lore raw

Hi Nick,
again thanks for your response in the 'arcmsr: code cleanup and some corrections' thread.

Let us find a resolution here too.
Is the original patch fine for you, or do you prefer not to take it?

Tomas

Quoted 42 lines · FUJITA Tomonori

Quoted 21 lines
Quoted 8 lines · On Thu, 17 Feb 2011 16:05:37 +0100 Tomas Henzl <[email protected]> wrote:
the discussion is mostly if 'dma_alloc_coherent'
...
dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size, &dma_coherent_handle, GFP_KERNEL);
...
returns a page address - dma_coherent with last, I think five bits, zeroed.
Could you help us with that?
Documentation/DMA-API-HOWTO.txt is not clear enough?
=
dma_alloc_coherent returns two values: the virtual address which you can use to access it from the CPU and dma_handle which you pass to the card.
Quoted 12 lines
The cpu return address and the DMA bus master address are both guaranteed to be aligned to the smallest PAGE_SIZE order which is greater than or equal to the requested size. This invariant exists (for example) to guarantee that if you allocate a chunk which is smaller than or equal to 64 kilobytes, the extent of the buffer you receive will not cross a 64K boundary.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
It's clear enough I think, I asked this question because Nick doesn't have a "100% confidence in that.". So I wanted a help to convince him.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

NickCheng

Acked-by

2011-02-25 01:31 UTC #7 lore raw

Acked-by Nick Cheng<[email protected]>

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Thursday, February 24, 2011 10:15 PM
To: FUJITA Tomonori
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 2/2] arcmsr: code cleanup and some corrections

Hi Nick,
again thanks for your response in the 'arcmsr: code cleanup and some corrections' thread.

Let us find a resolution here too.
Is the original patch fine for you, or do you prefer not to take it?

Tomas

Quoted 10 lines · FUJITA Tomonori

Quoted 9 lines
Quoted 3 lines · On Thu, 17 Feb 2011 16:05:37 +0100 Tomas Henzl <[email protected]> wrote:
the discussion is mostly if 'dma_alloc_coherent'
...
dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size,

&dma_coherent_handle, GFP_KERNEL);

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
...
returns a page address - dma_coherent with last, I think five bits,

zeroed.

Quoted 30 lines

Quoted 10 lines
Quoted 3 lines
Could you help us with that?
Documentation/DMA-API-HOWTO.txt is not clear enough?
=
dma_alloc_coherent returns two values: the virtual address which you can use to access it from the CPU and dma_handle which you pass to the card.
Quoted 12 lines
The cpu return address and the DMA bus master address are both guaranteed to be aligned to the smallest PAGE_SIZE order which is greater than or equal to the requested size. This invariant exists (for example) to guarantee that if you allocate a chunk which is smaller than or equal to 64 kilobytes, the extent of the buffer you receive will not cross a 64K boundary.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
It's clear enough I think, I asked this question because Nick doesn't have a "100% confidence in that.". So I wanted a help to convince him.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Tomas Henzl

author

2011-02-17 15:07 UTC #8 lore raw

In reply to Tomas Henzl

Hi Nick,

any comments here?

tomash

Quoted 204 lines

Signed-off-by Tomas henzl <[email protected]>

drivers/scsi/arcmsr/arcmsr_hba.c +48 ~~-71~~

index 984bd52..4cd522b 100644
@@ -75,8 +75,10 @@ MODULE_AUTHOR("Nick Cheng <[email protected]>");
 MODULE_DESCRIPTION("ARECA (ARC11xx/12xx/16xx/1880) SATA/SAS RAID Host Bus Adapter");
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(ARCMSR_DRIVER_VERSION);
-static int sleeptime = 10;
-static int retrycount = 12;
+
+#define	ARCMSR_SLEEPTIME	10
+#define	ARCMSR_RETRYCOUNT	12
+
 wait_queue_head_t wait_q;
 static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
 					struct scsi_cmnd *cmd);

@@ -171,24 +173,6 @@ static struct pci_driver arcmsr_pci_driver = {
 ****************************************************************************
 ****************************************************************************
 */
-int arcmsr_sleep_for_bus_reset(struct scsi_cmnd *cmd)
-{
-		struct Scsi_Host *shost = NULL;
-		int i, isleep;
-		shost = cmd->device->host;
-		isleep = sleeptime / 10;
-		if (isleep > 0) {
-			for (i = 0; i < isleep; i++) {
-				msleep(10000);
-			}
-		}
-
-		isleep = sleeptime % 10;
-		if (isleep > 0) {
-			msleep(isleep*1000);
-		}
-		return 0;
-}
 
 static void arcmsr_free_hbb_mu(struct AdapterControlBlock *acb)
 {

@@ -323,66 +307,64 @@ static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
 
 	default: acb->adapter_type = ACB_ADAPTER_TYPE_A;
 	}
-}	
+}
 
 static uint8_t arcmsr_hba_wait_msgint_ready(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&reg->outbound_intstatus) &
-					ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
-				writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
-					&reg->outbound_intstatus);
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&reg->outbound_intstatus) &
+				ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
+			writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
+				&reg->outbound_intstatus);
+			return true;
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbb_wait_msgint_ready(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_B *reg = acb->pmuB;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(reg->iop2drv_doorbell)
-				& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
-				writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN
-					, reg->iop2drv_doorbell);
-				writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT, reg->drv2iop_doorbell);
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(reg->iop2drv_doorbell)
+			& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
+			writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN,
+					reg->iop2drv_doorbell);
+			writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
+					reg->drv2iop_doorbell);
+			return true;
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbc_wait_msgint_ready(struct AdapterControlBlock *pACB)
 {
 	struct MessageUnit_C *phbcmu = (struct MessageUnit_C *)pACB->pmuC;
-	unsigned char Retries = 0x00;
-	uint32_t Index;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&phbcmu->outbound_doorbell) & ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
-				writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR, &phbcmu->outbound_doorbell_clear);/*clear interrupt*/
-				return true;
-			}
-			/* one us delay	*/
-			msleep(10);
-		} /*max 1 seconds*/
-	} while (Retries++ < 20); /*max 20 sec*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&phbcmu->outbound_doorbell)
+				& ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
+			writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR,
+				&phbcmu->outbound_doorbell_clear); /*clear interrupt*/
+			return true;
+		}
+		msleep(10);
+	} /* max 20 seconds */
+
 	return false;
 }
+
 static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;

@@ -2602,12 +2584,8 @@ static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
 		if (cdb_phyaddr_hi32 != 0) {
 			struct MessageUnit_C *reg = (struct MessageUnit_C *)acb->pmuC;
 
-			if (cdb_phyaddr_hi32 != 0) {
-				unsigned char Retries = 0x00;
-				do {
-					printk(KERN_NOTICE "arcmsr%d: cdb_phyaddr_hi32=0x%x \n", acb->adapter_index, cdb_phyaddr_hi32);
-				} while (Retries++ < 100);
-			}
+			printk(KERN_NOTICE "arcmsr%d: cdb_phyaddr_hi32=0x%x \n",
+					acb->adapter_index, cdb_phyaddr_hi32);
 			writel(ARCMSR_SIGNATURE_SET_CONFIG, &reg->msgcode_rwbuffer[0]);
 			writel(cdb_phyaddr_hi32, &reg->msgcode_rwbuffer[1]);
 			writel(ARCMSR_INBOUND_MESG0_SET_CONFIG, &reg->inbound_msgaddr0);

@@ -2955,10 +2933,10 @@ static int arcmsr_bus_reset(struct scsi_cmnd *cmd)
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep_again:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->outbound_msgaddr1) & ARCMSR_OUTBOUND_MESG1_FIRMWARE_OK) == 0) {
 					printk(KERN_ERR "arcmsr%d: waiting for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT) {
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d: waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
 						return FAILED;

@@ -3025,10 +3003,10 @@ sleep_again:
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->host_diagnostic) & 0x04) != 0) {
 					printk(KERN_ERR "arcmsr%d: waiting for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT) {
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d: waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
 						return FAILED;
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

NickCheng

2011-02-18 01:27 UTC #9 lore raw

Hi Tomas,
I have not yet tested this code.
Do you try that?

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Thursday, February 17, 2011 11:08 PM
To: '[email protected]'
Cc: [email protected]
Subject: Re: [PATCH 1/2] arcmsr: code cleanup and some corrections

Hi Nick,

any comments here?

tomash

Quoted 5 lines

I removed outer loops in ...wait_msgint_ready
the sleeptime and retrycount are in fact never changed so I changed them into defines. In arcmsr_flush_hba_cache is a loop removed, which printed the same printk 100 times, one line in log is enough I think. The arcmsr_sleep_for_bus_reset has lost a functionality with the latest

patches,

Quoted 1 line

The only thing the function does is a long sleep, so it's replaced with a

ssleep.

Review

Quoted 4 lines

Signed-off-by Tomas henzl <[email protected]>
(quoted diff) +0 -0

b/drivers/scsi/arcmsr/arcmsr_hba.c

drivers/scsi/arcmsr/arcmsr_hba.c MODULE_AUTHOR("Nick Cheng <[email protected]>"); · line 75

Quoted 5 lines

index 984bd52..4cd522b 100644
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -75,8 +75,10 @@ MODULE_AUTHOR("Nick Cheng <[email protected]>");
 MODULE_DESCRIPTION("ARECA (ARC11xx/12xx/16xx/1880) SATA/SAS RAID Host Bus

Adapter");

drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 173

Quoted 13 lines

drivers/scsi/arcmsr/arcmsr_hba.c +4 -2

 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(ARCMSR_DRIVER_VERSION);
-static int sleeptime = 10;
-static int retrycount = 12;
+
+#define	ARCMSR_SLEEPTIME	10
+#define	ARCMSR_RETRYCOUNT	12
+
 wait_queue_head_t wait_q;
 static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
 					struct scsi_cmnd *cmd);

@@ -171,24 +173,6 @@ static struct pci_driver arcmsr_pci_driver = {

****************************************************************************

Quoted 1 line

****************************************************************************

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct

Quoted 23 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 ~~-18~~

-int arcmsr_sleep_for_bus_reset(struct scsi_cmnd *cmd)
-{
-		struct Scsi_Host *shost = NULL;
-		int i, isleep;
-		shost = cmd->device->host;
-		isleep = sleeptime / 10;
-		if (isleep > 0) {
-			for (i = 0; i < isleep; i++) {
-				msleep(10000);
-			}
-		}
-
-		isleep = sleeptime % 10;
-		if (isleep > 0) {
-			msleep(isleep*1000);
-		}
-		return 0;
-}
 
 static void arcmsr_free_hbb_mu(struct AdapterControlBlock *acb)
 {

@@ -323,66 +307,64 @@ static void arcmsr_define_adapter_type(struct

AdapterControlBlock *acb)

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)

Quoted 7 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -1

 	default: acb->adapter_type = ACB_ADAPTER_TYPE_A;
 	}
-}	
+}
 
 static uint8_t arcmsr_hba_wait_msgint_ready(struct AdapterControlBlock

*acb)

Quoted 21 lines

drivers/scsi/arcmsr/arcmsr_hba.c +7 ~~-12~~

 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&reg->outbound_intstatus) &
-					ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
-				writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
-					&reg->outbound_intstatus);
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&reg->outbound_intstatus) &
+				ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
+			writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
+				&reg->outbound_intstatus);

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)

Quoted 10 lines

drivers/scsi/arcmsr/arcmsr_hba.c +4 -1

+			return true;
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbb_wait_msgint_ready(struct AdapterControlBlock

*acb)

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)

Quoted 11 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -9

 {
 	struct MessageUnit_B *reg = acb->pmuB;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(reg->iop2drv_doorbell)
-				& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
-				writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN
-					, reg->iop2drv_doorbell);
-				writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,

reg->drv2iop_doorbell);

Quoted 13 lines

drivers/scsi/arcmsr/arcmsr_hba.c +9 -4

-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(reg->iop2drv_doorbell)
+			& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
+			writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN,
+					reg->iop2drv_doorbell);
+			writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
+					reg->drv2iop_doorbell);

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)

Quoted 10 lines

drivers/scsi/arcmsr/arcmsr_hba.c +4 -1

+			return true;
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbc_wait_msgint_ready(struct AdapterControlBlock

*pACB)

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)

Quoted 7 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -5

 {
 	struct MessageUnit_C *phbcmu = (struct MessageUnit_C *)pACB->pmuC;
-	unsigned char Retries = 0x00;
-	uint32_t Index;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&phbcmu->outbound_doorbell) &

ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {

drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 185

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +0 -1
-

writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR, &phbcmu->outbound_doorbell_clear);/*clear interrupt*/

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 364

Quoted 12 lines

drivers/scsi/arcmsr/arcmsr_hba.c +6 -6

-				return true;
-			}
-			/* one us delay	*/
-			msleep(10);
-		} /*max 1 seconds*/
-	} while (Retries++ < 20); /*max 20 sec*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&phbcmu->outbound_doorbell)
+				& ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
+

writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR,

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct · line 358

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+				&phbcmu->outbound_doorbell_clear); /*clear

interrupt*/

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct

Quoted 12 lines

drivers/scsi/arcmsr/arcmsr_hba.c +6 -0

+			return true;
+		}
+		msleep(10);
+	} /* max 20 seconds */
+
 	return false;
 }
+
 static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;

@@ -2602,12 +2584,8 @@ static int arcmsr_iop_confirm(struct

AdapterControlBlock *acb)

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2585

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0

 		if (cdb_phyaddr_hi32 != 0) {
 			struct MessageUnit_C *reg = (struct MessageUnit_C

*)acb->pmuC;

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)

Quoted 5 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -4

-			if (cdb_phyaddr_hi32 != 0) {
-				unsigned char Retries = 0x00;
-				do {
-					printk(KERN_NOTICE "arcmsr%d:

cdb_phyaddr_hi32=0x%x \n", acb->adapter_index, cdb_phyaddr_hi32);

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)

Quoted 3 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -2

-				} while (Retries++ < 100);
-			}
+			printk(KERN_NOTICE "arcmsr%d: cdb_phyaddr_hi32=0x%x

\n",

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2587

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+					acb->adapter_index,

cdb_phyaddr_hi32);

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2588

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 			writel(ARCMSR_SIGNATURE_SET_CONFIG,

&reg->msgcode_rwbuffer[0]);

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2590

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0

 			writel(cdb_phyaddr_hi32, &reg->msgcode_rwbuffer[1]);
 			writel(ARCMSR_INBOUND_MESG0_SET_CONFIG,

&reg->inbound_msgaddr0);

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2937

Quoted 7 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -1

@@ -2955,10 +2933,10 @@ static int arcmsr_bus_reset(struct scsi_cmnd *cmd)
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep_again:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->outbound_msgaddr1) &

ARCMSR_OUTBOUND_MESG1_FIRMWARE_OK) == 0) {

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2938

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting

for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2939

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -1

-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)

{

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2941

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0

 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:

waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);

drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3007

Quoted 8 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -1

 						return FAILED;

@@ -3025,10 +3003,10 @@ sleep_again:
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->host_diagnostic) & 0x04) !=

0) {

drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3008

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting

for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);

drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3009

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -1

-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)

{

drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3011

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0

 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:

waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);

Quoted 8 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -1
 						return FAILED;
 
 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Tomas Henzl

author

2011-02-18 11:04 UTC #10 lore raw

Quoted 4 lines · NickCheng

Hi Tomas,
I have not yet tested this code.
Do you try that?

Yes, I tried it, but only a very basic test. Most of the code paths in the patch weren't tested.

If you want I can post a set of individual changes, maybe this would be better readable?

Quoted 316 lines

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Thursday, February 17, 2011 11:08 PM
To: '[email protected]'
Cc: [email protected]
Subject: Re: [PATCH 1/2] arcmsr: code cleanup and some corrections
Hi Nick,
any comments here?
tomash
Quoted 6 lines
I removed outer loops in ...wait_msgint_ready
the sleeptime and retrycount are in fact never changed so I changed them into defines. In arcmsr_flush_hba_cache is a loop removed, which printed the same printk 100 times, one line in log is enough I think. The arcmsr_sleep_for_bus_reset has lost a functionality with the latest
patches,
Quoted 2 lines
The only thing the function does is a long sleep, so it's replaced with a
ssleep.
Review
Quoted 4 lines
Signed-off-by Tomas henzl <[email protected]>
(quoted diff) +0 -0
b/drivers/scsi/arcmsr/arcmsr_hba.c
drivers/scsi/arcmsr/arcmsr_hba.c MODULE_AUTHOR("Nick Cheng <[email protected]>"); · line 76
Quoted 6 lines
index 984bd52..4cd522b 100644
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -75,8 +75,10 @@ MODULE_AUTHOR("Nick Cheng <[email protected]>");
 MODULE_DESCRIPTION("ARECA (ARC11xx/12xx/16xx/1880) SATA/SAS RAID Host Bus
 
Adapter");
Quoted 12 lines
drivers/scsi/arcmsr/arcmsr_hba.c +4 -2
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(ARCMSR_DRIVER_VERSION);
-static int sleeptime = 10;
-static int retrycount = 12;
+
+#define	ARCMSR_SLEEPTIME	10
+#define	ARCMSR_RETRYCOUNT	12
+
 wait_queue_head_t wait_q;
 static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					struct scsi_cmnd *cmd);
drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 174
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -171,24 +173,6 @@ static struct pci_driver arcmsr_pci_driver = {
 
 
****************************************************************************
Quoted 1 line
****************************************************************************
Quoted 23 lines
*/
drivers/scsi/arcmsr/arcmsr_hba.c +0 ~~-18~~
-int arcmsr_sleep_for_bus_reset(struct scsi_cmnd *cmd)
-{
-		struct Scsi_Host *shost = NULL;
-		int i, isleep;
-		shost = cmd->device->host;
-		isleep = sleeptime / 10;
-		if (isleep > 0) {
-			for (i = 0; i < isleep; i++) {
-				msleep(10000);
-			}
-		}
-
-		isleep = sleeptime % 10;
-		if (isleep > 0) {
-			msleep(isleep*1000);
-		}
-		return 0;
-}
 
 static void arcmsr_free_hbb_mu(struct AdapterControlBlock *acb)
 {
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct · line 307
Quoted 1 line
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -323,66 +307,64 @@ static void arcmsr_define_adapter_type(struct
 
AdapterControlBlock *acb)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 326
Quoted 8 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
 	default: acb->adapter_type = ACB_ADAPTER_TYPE_A;
 	}
-}	
+}
 
 static uint8_t arcmsr_hba_wait_msgint_ready(struct AdapterControlBlock
 
*acb)
Quoted 22 lines
drivers/scsi/arcmsr/arcmsr_hba.c +8 ~~-12~~
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&reg->outbound_intstatus) &
-					ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
-				writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
-					&reg->outbound_intstatus);
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&reg->outbound_intstatus) &
+				ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
+			writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
+				&reg->outbound_intstatus);
+			return true;
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 346
Quoted 10 lines
drivers/scsi/arcmsr/arcmsr_hba.c +3 -1
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbb_wait_msgint_ready(struct AdapterControlBlock
 
*acb)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 346
Quoted 12 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -9
 {
 	struct MessageUnit_B *reg = acb->pmuB;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(reg->iop2drv_doorbell)
-				& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
-				writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN
-					, reg->iop2drv_doorbell);
-				writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
 
reg->drv2iop_doorbell);
Quoted 14 lines
drivers/scsi/arcmsr/arcmsr_hba.c +10 -4
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(reg->iop2drv_doorbell)
+			& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
+			writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN,
+					reg->iop2drv_doorbell);
+			writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
+					reg->drv2iop_doorbell);
+			return true;
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
Quoted 10 lines
drivers/scsi/arcmsr/arcmsr_hba.c +3 -1
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbc_wait_msgint_ready(struct AdapterControlBlock
 
*pACB)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
Quoted 8 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -5
 {
 	struct MessageUnit_C *phbcmu = (struct MessageUnit_C *)pACB->pmuC;
-	unsigned char Retries = 0x00;
-	uint32_t Index;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&phbcmu->outbound_doorbell) &
 
ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 176
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -1
-
 
writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR, &phbcmu->outbound_doorbell_clear);/*clear interrupt*/
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 364
Quoted 13 lines
drivers/scsi/arcmsr/arcmsr_hba.c +6 -6
-				return true;
-			}
-			/* one us delay	*/
-			msleep(10);
-		} /*max 1 seconds*/
-	} while (Retries++ < 20); /*max 20 sec*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&phbcmu->outbound_doorbell)
+				& ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
+
writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR,
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+				&phbcmu->outbound_doorbell_clear); /*clear
interrupt*/
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2584
Quoted 13 lines
drivers/scsi/arcmsr/arcmsr_hba.c +6 -0
+			return true;
+		}
+		msleep(10);
+	} /* max 20 seconds */
+
 	return false;
 }
+
 static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
@@ -2602,12 +2584,8 @@ static int arcmsr_iop_confirm(struct
 
AdapterControlBlock *acb)
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb) · line 2586
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 		if (cdb_phyaddr_hi32 != 0) {
 			struct MessageUnit_C *reg = (struct MessageUnit_C
 
*)acb->pmuC;
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
Quoted 6 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -4
-			if (cdb_phyaddr_hi32 != 0) {
-				unsigned char Retries = 0x00;
-				do {
-					printk(KERN_NOTICE "arcmsr%d:
 
cdb_phyaddr_hi32=0x%x \n", acb->adapter_index, cdb_phyaddr_hi32);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
Quoted 4 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -2
-				} while (Retries++ < 100);
-			}
+			printk(KERN_NOTICE "arcmsr%d: cdb_phyaddr_hi32=0x%x
 
\n",
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2591
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+					acb->adapter_index,
cdb_phyaddr_hi32);
Quoted 2 lines
writel(ARCMSR_SIGNATURE_SET_CONFIG,
&reg->msgcode_rwbuffer[0]);
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 			writel(cdb_phyaddr_hi32, &reg->msgcode_rwbuffer[1]);
 			writel(ARCMSR_INBOUND_MESG0_SET_CONFIG,
&reg->inbound_msgaddr0);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2938
Quoted 8 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
@@ -2955,10 +2933,10 @@ static int arcmsr_bus_reset(struct scsi_cmnd *cmd)
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep_again:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->outbound_msgaddr1) &
 
ARCMSR_OUTBOUND_MESG1_FIRMWARE_OK) == 0) {
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2940
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting
 
for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2942
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)
 
{
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:
waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3008
Quoted 9 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
 						return FAILED;
@@ -3025,10 +3003,10 @@ sleep_again:
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->host_diagnostic) & 0x04) !=
 
0) {
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3010
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting
 
for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3012
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)
 
{
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:
waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
Quoted 9 lines
return FAILED;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

NickCheng

2011-02-18 11:38 UTC #11 lore raw

Hi Tomas,
I will make time to test it.
Please give me few days.

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Friday, February 18, 2011 7:04 PM
To: NickCheng
Cc: [email protected]
Subject: Re: [PATCH 1/2] arcmsr: code cleanup and some corrections

Quoted 4 lines · NickCheng

Hi Tomas,
I have not yet tested this code.
Do you try that?

Yes, I tried it, but only a very basic test. Most of the code paths in the patch weren't tested.

If you want I can post a set of individual changes, maybe this would be better readable?

Quoted 33 lines

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Thursday, February 17, 2011 11:08 PM
To: '[email protected]'
Cc: [email protected]
Subject: Re: [PATCH 1/2] arcmsr: code cleanup and some corrections
Hi Nick,
any comments here?
tomash
Quoted 6 lines
I removed outer loops in ...wait_msgint_ready
the sleeptime and retrycount are in fact never changed so I changed them into defines. In arcmsr_flush_hba_cache is a loop removed, which printed the same printk 100 times, one line in log is enough I think. The arcmsr_sleep_for_bus_reset has lost a functionality with the latest
patches,
Quoted 2 lines
The only thing the function does is a long sleep, so it's replaced with a
ssleep.
Review
Quoted 4 lines
Signed-off-by Tomas henzl <[email protected]>
(quoted diff) +0 -0
b/drivers/scsi/arcmsr/arcmsr_hba.c

Quoted 5 lines

drivers/scsi/arcmsr/arcmsr_hba.c MODULE_AUTHOR("Nick Cheng <[email protected]>"); · line 75
Quoted 5 lines
index 984bd52..4cd522b 100644
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -75,8 +75,10 @@ MODULE_AUTHOR("Nick Cheng <[email protected]>");
 MODULE_DESCRIPTION("ARECA (ARC11xx/12xx/16xx/1880) SATA/SAS RAID Host

Bus

Quoted 18 lines

Quoted 1 line
Adapter");
Quoted 12 lines
MODULE_LICENSE("Dual BSD/GPL");
MODULE_VERSION(ARCMSR_DRIVER_VERSION);
drivers/scsi/arcmsr/arcmsr_hba.c +4 -2
-static int sleeptime = 10;
-static int retrycount = 12;
+
+#define	ARCMSR_SLEEPTIME	10
+#define	ARCMSR_RETRYCOUNT	12
+
 wait_queue_head_t wait_q;
 static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
 					struct scsi_cmnd *cmd);
drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 174
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -171,24 +173,6 @@ static struct pci_driver arcmsr_pci_driver = {
 
 

****************************************************************************

Quoted 3 lines

Quoted 2 lines

****************************************************************************

Quoted 185 lines

Quoted 24 lines
*/
drivers/scsi/arcmsr/arcmsr_hba.c +0 ~~-18~~
-int arcmsr_sleep_for_bus_reset(struct scsi_cmnd *cmd)
-{
-		struct Scsi_Host *shost = NULL;
-		int i, isleep;
-		shost = cmd->device->host;
-		isleep = sleeptime / 10;
-		if (isleep > 0) {
-			for (i = 0; i < isleep; i++) {
-				msleep(10000);
-			}
-		}
-
-		isleep = sleeptime % 10;
-		if (isleep > 0) {
-			msleep(isleep*1000);
-		}
-		return 0;
-}
 
 static void arcmsr_free_hbb_mu(struct AdapterControlBlock *acb)
 {
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct · line 307
Quoted 1 line
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -323,66 +307,64 @@ static void arcmsr_define_adapter_type(struct
 
AdapterControlBlock *acb)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 326
Quoted 8 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
 	default: acb->adapter_type = ACB_ADAPTER_TYPE_A;
 	}
-}	
+}
 
 static uint8_t arcmsr_hba_wait_msgint_ready(struct AdapterControlBlock
 
*acb)
Quoted 22 lines
drivers/scsi/arcmsr/arcmsr_hba.c +8 ~~-12~~
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&reg->outbound_intstatus) &
-					ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
-				writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
-					&reg->outbound_intstatus);
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&reg->outbound_intstatus) &
+				ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
+			writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
+				&reg->outbound_intstatus);
+			return true;
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 346
Quoted 10 lines
drivers/scsi/arcmsr/arcmsr_hba.c +3 -1
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbb_wait_msgint_ready(struct AdapterControlBlock
 
*acb)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 346
Quoted 12 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -9
 {
 	struct MessageUnit_B *reg = acb->pmuB;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(reg->iop2drv_doorbell)
-				& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
-				writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN
-					, reg->iop2drv_doorbell);
-				writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
 
reg->drv2iop_doorbell);
Quoted 14 lines
drivers/scsi/arcmsr/arcmsr_hba.c +10 -4
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(reg->iop2drv_doorbell)
+			& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
+			writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN,
+					reg->iop2drv_doorbell);
+			writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
+					reg->drv2iop_doorbell);
+			return true;
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
Quoted 10 lines
drivers/scsi/arcmsr/arcmsr_hba.c +3 -1
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbc_wait_msgint_ready(struct AdapterControlBlock
 
*pACB)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
Quoted 8 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -5
 {
 	struct MessageUnit_C *phbcmu = (struct MessageUnit_C *)pACB->pmuC;
-	unsigned char Retries = 0x00;
-	uint32_t Index;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&phbcmu->outbound_doorbell) &
 
ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 176
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -1
-
 
writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR, &phbcmu->outbound_doorbell_clear);/*clear interrupt*/
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 364
Quoted 13 lines
drivers/scsi/arcmsr/arcmsr_hba.c +6 -6
-				return true;
-			}
-			/* one us delay	*/
-			msleep(10);
-		} /*max 1 seconds*/
-	} while (Retries++ < 20); /*max 20 sec*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&phbcmu->outbound_doorbell)
+				& ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
+
writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR,
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+				&phbcmu->outbound_doorbell_clear); /*clear
interrupt*/
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2584
Quoted 13 lines
drivers/scsi/arcmsr/arcmsr_hba.c +6 -0
+			return true;
+		}
+		msleep(10);
+	} /* max 20 seconds */
+
 	return false;
 }
+
 static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
@@ -2602,12 +2584,8 @@ static int arcmsr_iop_confirm(struct
 
AdapterControlBlock *acb)
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb) · line 2586
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 		if (cdb_phyaddr_hi32 != 0) {
 			struct MessageUnit_C *reg = (struct MessageUnit_C
 
*)acb->pmuC;
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
Quoted 6 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -4
-			if (cdb_phyaddr_hi32 != 0) {
-				unsigned char Retries = 0x00;
-				do {
-					printk(KERN_NOTICE "arcmsr%d:
 
cdb_phyaddr_hi32=0x%x \n", acb->adapter_index, cdb_phyaddr_hi32);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
Quoted 4 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -2
-				} while (Retries++ < 100);
-			}
+			printk(KERN_NOTICE "arcmsr%d: cdb_phyaddr_hi32=0x%x
 
\n",
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2591
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+					acb->adapter_index,
cdb_phyaddr_hi32);
Quoted 2 lines
writel(ARCMSR_SIGNATURE_SET_CONFIG,

Quoted 8 lines

&reg->msgcode_rwbuffer[0]);
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 			writel(cdb_phyaddr_hi32, &reg->msgcode_rwbuffer[1]);
 			writel(ARCMSR_INBOUND_MESG0_SET_CONFIG,
&reg->inbound_msgaddr0);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd
Quoted 1 line
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -2955,10 +2933,10 @@ static int arcmsr_bus_reset(struct scsi_cmnd

*cmd)

Quoted 64 lines

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd)
Quoted 7 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep_again:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->outbound_msgaddr1) &
 
ARCMSR_OUTBOUND_MESG1_FIRMWARE_OK) == 0) {
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd · line 2940
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting
 
for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd · line 2942
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)
 
{
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:
waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3008
Quoted 9 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
 						return FAILED;
@@ -3025,10 +3003,10 @@ sleep_again:
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->host_diagnostic) & 0x04) !=
 
0) {
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3010
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting
 
for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3012
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)
 
{
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:
waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
Quoted 9 lines
return FAILED;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

NickCheng

2011-02-24 02:04 UTC #12 lore raw

In reply to Tomas Henzl

Hi Tomas,
The test on my site is so far so good.

-----Original Message-----
From: NickCheng [mailto:[email protected]]
Sent: Friday, February 18, 2011 9:26 AM
To: 'Tomas Henzl'; '[email protected]'
Subject: RE: [PATCH 1/2] arcmsr: code cleanup and some corrections

Hi Tomas,
I have not yet tested this code.
Do you try that?

Hi Nick,

any comments here?

tomash

Quoted 5 lines

I removed outer loops in ...wait_msgint_ready
the sleeptime and retrycount are in fact never changed so I changed them into defines. In arcmsr_flush_hba_cache is a loop removed, which printed the same printk 100 times, one line in log is enough I think. The arcmsr_sleep_for_bus_reset has lost a functionality with the latest

patches,

Quoted 1 line

The only thing the function does is a long sleep, so it's replaced with a

ssleep.

Review

Quoted 4 lines

Signed-off-by Tomas henzl <[email protected]>
(quoted diff) +0 -0

b/drivers/scsi/arcmsr/arcmsr_hba.c

drivers/scsi/arcmsr/arcmsr_hba.c MODULE_AUTHOR("Nick Cheng <[email protected]>"); · line 75

Quoted 5 lines

index 984bd52..4cd522b 100644
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -75,8 +75,10 @@ MODULE_AUTHOR("Nick Cheng <[email protected]>");
 MODULE_DESCRIPTION("ARECA (ARC11xx/12xx/16xx/1880) SATA/SAS RAID Host Bus

Adapter");

drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 173

Quoted 13 lines

drivers/scsi/arcmsr/arcmsr_hba.c +4 -2

 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(ARCMSR_DRIVER_VERSION);
-static int sleeptime = 10;
-static int retrycount = 12;
+
+#define	ARCMSR_SLEEPTIME	10
+#define	ARCMSR_RETRYCOUNT	12
+
 wait_queue_head_t wait_q;
 static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
 					struct scsi_cmnd *cmd);

@@ -171,24 +173,6 @@ static struct pci_driver arcmsr_pci_driver = {

****************************************************************************

Quoted 1 line

****************************************************************************

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct

Quoted 23 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 ~~-18~~

-int arcmsr_sleep_for_bus_reset(struct scsi_cmnd *cmd)
-{
-		struct Scsi_Host *shost = NULL;
-		int i, isleep;
-		shost = cmd->device->host;
-		isleep = sleeptime / 10;
-		if (isleep > 0) {
-			for (i = 0; i < isleep; i++) {
-				msleep(10000);
-			}
-		}
-
-		isleep = sleeptime % 10;
-		if (isleep > 0) {
-			msleep(isleep*1000);
-		}
-		return 0;
-}
 
 static void arcmsr_free_hbb_mu(struct AdapterControlBlock *acb)
 {

@@ -323,66 +307,64 @@ static void arcmsr_define_adapter_type(struct

AdapterControlBlock *acb)

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)

Quoted 7 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -1

 	default: acb->adapter_type = ACB_ADAPTER_TYPE_A;
 	}
-}	
+}
 
 static uint8_t arcmsr_hba_wait_msgint_ready(struct AdapterControlBlock

*acb)

Quoted 21 lines

drivers/scsi/arcmsr/arcmsr_hba.c +7 ~~-12~~

 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&reg->outbound_intstatus) &
-					ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
-				writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
-					&reg->outbound_intstatus);
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&reg->outbound_intstatus) &
+				ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
+			writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
+				&reg->outbound_intstatus);

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)

Quoted 10 lines

drivers/scsi/arcmsr/arcmsr_hba.c +4 -1

+			return true;
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbb_wait_msgint_ready(struct AdapterControlBlock

*acb)

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)

Quoted 11 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -9

 {
 	struct MessageUnit_B *reg = acb->pmuB;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(reg->iop2drv_doorbell)
-				& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
-				writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN
-					, reg->iop2drv_doorbell);
-				writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,

reg->drv2iop_doorbell);

Quoted 13 lines

drivers/scsi/arcmsr/arcmsr_hba.c +9 -4

-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(reg->iop2drv_doorbell)
+			& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
+			writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN,
+					reg->iop2drv_doorbell);
+			writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
+					reg->drv2iop_doorbell);

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)

Quoted 10 lines

drivers/scsi/arcmsr/arcmsr_hba.c +4 -1

+			return true;
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbc_wait_msgint_ready(struct AdapterControlBlock

*pACB)

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)

Quoted 7 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -5

 {
 	struct MessageUnit_C *phbcmu = (struct MessageUnit_C *)pACB->pmuC;
-	unsigned char Retries = 0x00;
-	uint32_t Index;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&phbcmu->outbound_doorbell) &

ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {

drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 185

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +0 -1
-

writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR, &phbcmu->outbound_doorbell_clear);/*clear interrupt*/

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 364

Quoted 12 lines

drivers/scsi/arcmsr/arcmsr_hba.c +6 -6

-				return true;
-			}
-			/* one us delay	*/
-			msleep(10);
-		} /*max 1 seconds*/
-	} while (Retries++ < 20); /*max 20 sec*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&phbcmu->outbound_doorbell)
+				& ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
+

writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR,

drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct · line 358

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+				&phbcmu->outbound_doorbell_clear); /*clear

interrupt*/

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct

Quoted 12 lines

drivers/scsi/arcmsr/arcmsr_hba.c +6 -0

+			return true;
+		}
+		msleep(10);
+	} /* max 20 seconds */
+
 	return false;
 }
+
 static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;

@@ -2602,12 +2584,8 @@ static int arcmsr_iop_confirm(struct

AdapterControlBlock *acb)

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2585

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0

 		if (cdb_phyaddr_hi32 != 0) {
 			struct MessageUnit_C *reg = (struct MessageUnit_C

*)acb->pmuC;

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)

Quoted 5 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -4

-			if (cdb_phyaddr_hi32 != 0) {
-				unsigned char Retries = 0x00;
-				do {
-					printk(KERN_NOTICE "arcmsr%d:

cdb_phyaddr_hi32=0x%x \n", acb->adapter_index, cdb_phyaddr_hi32);

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)

Quoted 3 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -2

-				} while (Retries++ < 100);
-			}
+			printk(KERN_NOTICE "arcmsr%d: cdb_phyaddr_hi32=0x%x

\n",

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2587

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+					acb->adapter_index,

cdb_phyaddr_hi32);

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2588

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 			writel(ARCMSR_SIGNATURE_SET_CONFIG,

&reg->msgcode_rwbuffer[0]);

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2590

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0

 			writel(cdb_phyaddr_hi32, &reg->msgcode_rwbuffer[1]);
 			writel(ARCMSR_INBOUND_MESG0_SET_CONFIG,

&reg->inbound_msgaddr0);

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2937

Quoted 7 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -1

@@ -2955,10 +2933,10 @@ static int arcmsr_bus_reset(struct scsi_cmnd *cmd)
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep_again:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->outbound_msgaddr1) &

ARCMSR_OUTBOUND_MESG1_FIRMWARE_OK) == 0) {

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2938

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting

for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2939

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -1

-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)

{

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2941

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0

 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:

waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);

drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3007

Quoted 8 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -1

 						return FAILED;

@@ -3025,10 +3003,10 @@ sleep_again:
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->host_diagnostic) & 0x04) !=

0) {

drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3008

Quoted 1 line

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting

for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);

drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3009

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +1 -1

-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)

{

drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3011

Quoted 2 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -0

 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:

waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);

Quoted 8 lines

drivers/scsi/arcmsr/arcmsr_hba.c +0 -1
 						return FAILED;
 
 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Tomas Henzl

author

2011-02-24 14:09 UTC #13 lore raw

Quoted 3 lines · NickCheng

Hi Tomas,
The test on my site is so far so good.

Thanks Nick,
if this means you accept this patch please
add an 'Acked-by: ...' line so it is clear what you mean.

Quoted 326 lines

-----Original Message-----
From: NickCheng [mailto:[email protected]]
Sent: Friday, February 18, 2011 9:26 AM
To: 'Tomas Henzl'; '[email protected]'
Subject: RE: [PATCH 1/2] arcmsr: code cleanup and some corrections
Hi Tomas,
I have not yet tested this code.
Do you try that?
-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Thursday, February 17, 2011 11:08 PM
To: '[email protected]'
Cc: [email protected]
Subject: Re: [PATCH 1/2] arcmsr: code cleanup and some corrections
Hi Nick,
any comments here?
tomash
Quoted 6 lines
I removed outer loops in ...wait_msgint_ready
the sleeptime and retrycount are in fact never changed so I changed them into defines. In arcmsr_flush_hba_cache is a loop removed, which printed the same printk 100 times, one line in log is enough I think. The arcmsr_sleep_for_bus_reset has lost a functionality with the latest
patches,
Quoted 2 lines
The only thing the function does is a long sleep, so it's replaced with a
ssleep.
Review
Quoted 4 lines
Signed-off-by Tomas henzl <[email protected]>
(quoted diff) +0 -0
b/drivers/scsi/arcmsr/arcmsr_hba.c
drivers/scsi/arcmsr/arcmsr_hba.c MODULE_AUTHOR("Nick Cheng <[email protected]>"); · line 76
Quoted 6 lines
index 984bd52..4cd522b 100644
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -75,8 +75,10 @@ MODULE_AUTHOR("Nick Cheng <[email protected]>");
 MODULE_DESCRIPTION("ARECA (ARC11xx/12xx/16xx/1880) SATA/SAS RAID Host Bus
 
Adapter");
Quoted 12 lines
drivers/scsi/arcmsr/arcmsr_hba.c +4 -2
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(ARCMSR_DRIVER_VERSION);
-static int sleeptime = 10;
-static int retrycount = 12;
+
+#define	ARCMSR_SLEEPTIME	10
+#define	ARCMSR_RETRYCOUNT	12
+
 wait_queue_head_t wait_q;
 static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					struct scsi_cmnd *cmd);
drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 174
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -171,24 +173,6 @@ static struct pci_driver arcmsr_pci_driver = {
 
 
****************************************************************************
Quoted 1 line
****************************************************************************
Quoted 23 lines
*/
drivers/scsi/arcmsr/arcmsr_hba.c +0 ~~-18~~
-int arcmsr_sleep_for_bus_reset(struct scsi_cmnd *cmd)
-{
-		struct Scsi_Host *shost = NULL;
-		int i, isleep;
-		shost = cmd->device->host;
-		isleep = sleeptime / 10;
-		if (isleep > 0) {
-			for (i = 0; i < isleep; i++) {
-				msleep(10000);
-			}
-		}
-
-		isleep = sleeptime % 10;
-		if (isleep > 0) {
-			msleep(isleep*1000);
-		}
-		return 0;
-}
 
 static void arcmsr_free_hbb_mu(struct AdapterControlBlock *acb)
 {
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct · line 307
Quoted 1 line
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -323,66 +307,64 @@ static void arcmsr_define_adapter_type(struct
 
AdapterControlBlock *acb)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 326
Quoted 8 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
 	default: acb->adapter_type = ACB_ADAPTER_TYPE_A;
 	}
-}	
+}
 
 static uint8_t arcmsr_hba_wait_msgint_ready(struct AdapterControlBlock
 
*acb)
Quoted 22 lines
drivers/scsi/arcmsr/arcmsr_hba.c +8 ~~-12~~
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&reg->outbound_intstatus) &
-					ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
-				writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
-					&reg->outbound_intstatus);
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&reg->outbound_intstatus) &
+				ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
+			writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
+				&reg->outbound_intstatus);
+			return true;
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 346
Quoted 10 lines
drivers/scsi/arcmsr/arcmsr_hba.c +3 -1
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbb_wait_msgint_ready(struct AdapterControlBlock
 
*acb)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 346
Quoted 12 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -9
 {
 	struct MessageUnit_B *reg = acb->pmuB;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(reg->iop2drv_doorbell)
-				& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
-				writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN
-					, reg->iop2drv_doorbell);
-				writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
 
reg->drv2iop_doorbell);
Quoted 14 lines
drivers/scsi/arcmsr/arcmsr_hba.c +10 -4
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(reg->iop2drv_doorbell)
+			& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
+			writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN,
+					reg->iop2drv_doorbell);
+			writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
+					reg->drv2iop_doorbell);
+			return true;
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
Quoted 10 lines
drivers/scsi/arcmsr/arcmsr_hba.c +3 -1
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbc_wait_msgint_ready(struct AdapterControlBlock
 
*pACB)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
Quoted 8 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -5
 {
 	struct MessageUnit_C *phbcmu = (struct MessageUnit_C *)pACB->pmuC;
-	unsigned char Retries = 0x00;
-	uint32_t Index;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&phbcmu->outbound_doorbell) &
 
ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 176
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -1
-
 
writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR, &phbcmu->outbound_doorbell_clear);/*clear interrupt*/
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 364
Quoted 13 lines
drivers/scsi/arcmsr/arcmsr_hba.c +6 -6
-				return true;
-			}
-			/* one us delay	*/
-			msleep(10);
-		} /*max 1 seconds*/
-	} while (Retries++ < 20); /*max 20 sec*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&phbcmu->outbound_doorbell)
+				& ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
+
writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR,
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+				&phbcmu->outbound_doorbell_clear); /*clear
interrupt*/
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2584
Quoted 13 lines
drivers/scsi/arcmsr/arcmsr_hba.c +6 -0
+			return true;
+		}
+		msleep(10);
+	} /* max 20 seconds */
+
 	return false;
 }
+
 static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
@@ -2602,12 +2584,8 @@ static int arcmsr_iop_confirm(struct
 
AdapterControlBlock *acb)
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb) · line 2586
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 		if (cdb_phyaddr_hi32 != 0) {
 			struct MessageUnit_C *reg = (struct MessageUnit_C
 
*)acb->pmuC;
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
Quoted 6 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -4
-			if (cdb_phyaddr_hi32 != 0) {
-				unsigned char Retries = 0x00;
-				do {
-					printk(KERN_NOTICE "arcmsr%d:
 
cdb_phyaddr_hi32=0x%x \n", acb->adapter_index, cdb_phyaddr_hi32);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
Quoted 4 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -2
-				} while (Retries++ < 100);
-			}
+			printk(KERN_NOTICE "arcmsr%d: cdb_phyaddr_hi32=0x%x
 
\n",
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2591
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+					acb->adapter_index,
cdb_phyaddr_hi32);
Quoted 2 lines
writel(ARCMSR_SIGNATURE_SET_CONFIG,
&reg->msgcode_rwbuffer[0]);
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 			writel(cdb_phyaddr_hi32, &reg->msgcode_rwbuffer[1]);
 			writel(ARCMSR_INBOUND_MESG0_SET_CONFIG,
&reg->inbound_msgaddr0);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2938
Quoted 8 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
@@ -2955,10 +2933,10 @@ static int arcmsr_bus_reset(struct scsi_cmnd *cmd)
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep_again:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->outbound_msgaddr1) &
 
ARCMSR_OUTBOUND_MESG1_FIRMWARE_OK) == 0) {
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2940
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting
 
for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd) · line 2942
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)
 
{
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:
waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3008
Quoted 9 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
 						return FAILED;
@@ -3025,10 +3003,10 @@ sleep_again:
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->host_diagnostic) & 0x04) !=
 
0) {
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3010
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting
 
for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3012
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)
 
{
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:
waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
Quoted 9 lines
return FAILED;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

NickCheng

Acked-by

2011-02-25 01:31 UTC #14 lore raw

Acked-by Nick Cheng<[email protected]>

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Thursday, February 24, 2011 10:10 PM
To: NickCheng
Cc: [email protected]
Subject: Re: [PATCH 1/2] arcmsr: code cleanup and some corrections

Quoted 3 lines · NickCheng

Hi Tomas,
The test on my site is so far so good.

Thanks Nick,
if this means you accept this patch please
add an 'Acked-by: ...' line so it is clear what you mean.

Quoted 43 lines

-----Original Message-----
From: NickCheng [mailto:[email protected]]
Sent: Friday, February 18, 2011 9:26 AM
To: 'Tomas Henzl'; '[email protected]'
Subject: RE: [PATCH 1/2] arcmsr: code cleanup and some corrections
Hi Tomas,
I have not yet tested this code.
Do you try that?
-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Thursday, February 17, 2011 11:08 PM
To: '[email protected]'
Cc: [email protected]
Subject: Re: [PATCH 1/2] arcmsr: code cleanup and some corrections
Hi Nick,
any comments here?
tomash
Quoted 6 lines
I removed outer loops in ...wait_msgint_ready
the sleeptime and retrycount are in fact never changed so I changed them into defines. In arcmsr_flush_hba_cache is a loop removed, which printed the same printk 100 times, one line in log is enough I think. The arcmsr_sleep_for_bus_reset has lost a functionality with the latest
patches,
Quoted 2 lines
The only thing the function does is a long sleep, so it's replaced with a
ssleep.
Review
Quoted 4 lines
Signed-off-by Tomas henzl <[email protected]>
(quoted diff) +0 -0
b/drivers/scsi/arcmsr/arcmsr_hba.c

Quoted 5 lines

drivers/scsi/arcmsr/arcmsr_hba.c MODULE_AUTHOR("Nick Cheng <[email protected]>"); · line 75
Quoted 5 lines
index 984bd52..4cd522b 100644
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -75,8 +75,10 @@ MODULE_AUTHOR("Nick Cheng <[email protected]>");
 MODULE_DESCRIPTION("ARECA (ARC11xx/12xx/16xx/1880) SATA/SAS RAID Host

Bus

Quoted 18 lines

Quoted 1 line
Adapter");
Quoted 12 lines
MODULE_LICENSE("Dual BSD/GPL");
MODULE_VERSION(ARCMSR_DRIVER_VERSION);
drivers/scsi/arcmsr/arcmsr_hba.c +4 -2
-static int sleeptime = 10;
-static int retrycount = 12;
+
+#define	ARCMSR_SLEEPTIME	10
+#define	ARCMSR_RETRYCOUNT	12
+
 wait_queue_head_t wait_q;
 static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
 					struct scsi_cmnd *cmd);
drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 174
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -171,24 +173,6 @@ static struct pci_driver arcmsr_pci_driver = {
 
 

****************************************************************************

Quoted 3 lines

Quoted 2 lines

****************************************************************************

Quoted 185 lines

Quoted 24 lines
*/
drivers/scsi/arcmsr/arcmsr_hba.c +0 ~~-18~~
-int arcmsr_sleep_for_bus_reset(struct scsi_cmnd *cmd)
-{
-		struct Scsi_Host *shost = NULL;
-		int i, isleep;
-		shost = cmd->device->host;
-		isleep = sleeptime / 10;
-		if (isleep > 0) {
-			for (i = 0; i < isleep; i++) {
-				msleep(10000);
-			}
-		}
-
-		isleep = sleeptime % 10;
-		if (isleep > 0) {
-			msleep(isleep*1000);
-		}
-		return 0;
-}
 
 static void arcmsr_free_hbb_mu(struct AdapterControlBlock *acb)
 {
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct · line 307
Quoted 1 line
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -323,66 +307,64 @@ static void arcmsr_define_adapter_type(struct
 
AdapterControlBlock *acb)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 326
Quoted 8 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
 	default: acb->adapter_type = ACB_ADAPTER_TYPE_A;
 	}
-}	
+}
 
 static uint8_t arcmsr_hba_wait_msgint_ready(struct AdapterControlBlock
 
*acb)
Quoted 22 lines
drivers/scsi/arcmsr/arcmsr_hba.c +8 ~~-12~~
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&reg->outbound_intstatus) &
-					ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
-				writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
-					&reg->outbound_intstatus);
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&reg->outbound_intstatus) &
+				ARCMSR_MU_OUTBOUND_MESSAGE0_INT) {
+			writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT,
+				&reg->outbound_intstatus);
+			return true;
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 346
Quoted 10 lines
drivers/scsi/arcmsr/arcmsr_hba.c +3 -1
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbb_wait_msgint_ready(struct AdapterControlBlock
 
*acb)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 346
Quoted 12 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -9
 {
 	struct MessageUnit_B *reg = acb->pmuB;
-	uint32_t Index;
-	uint8_t Retries = 0x00;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(reg->iop2drv_doorbell)
-				& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
-				writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN
-					, reg->iop2drv_doorbell);
-				writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
 
reg->drv2iop_doorbell);
Quoted 14 lines
drivers/scsi/arcmsr/arcmsr_hba.c +10 -4
-				return true;
-			}
-			msleep(10);
-		}/*max 1 seconds*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(reg->iop2drv_doorbell)
+			& ARCMSR_IOP2DRV_MESSAGE_CMD_DONE) {
+			writel(ARCMSR_MESSAGE_INT_CLEAR_PATTERN,
+					reg->iop2drv_doorbell);
+			writel(ARCMSR_DRV2IOP_END_OF_INTERRUPT,
+					reg->drv2iop_doorbell);
+			return true;
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
Quoted 10 lines
drivers/scsi/arcmsr/arcmsr_hba.c +3 -1
+		}
+		msleep(10);
+	}/* max 20 seconds */
 
-	} while (Retries++ < 20);/*max 20 sec*/
 	return false;
 }
 
 static uint8_t arcmsr_hbc_wait_msgint_ready(struct AdapterControlBlock
 
*pACB)
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
Quoted 8 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -5
 {
 	struct MessageUnit_C *phbcmu = (struct MessageUnit_C *)pACB->pmuC;
-	unsigned char Retries = 0x00;
-	uint32_t Index;
-	do {
-		for (Index = 0; Index < 100; Index++) {
-			if (readl(&phbcmu->outbound_doorbell) &
 
ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
drivers/scsi/arcmsr/arcmsr_hba.c static struct pci_driver arcmsr_pci_driver = { · line 176
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -1
-
 
writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR, &phbcmu->outbound_doorbell_clear);/*clear interrupt*/
drivers/scsi/arcmsr/arcmsr_hba.c static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb) · line 364
Quoted 13 lines
drivers/scsi/arcmsr/arcmsr_hba.c +6 -6
-				return true;
-			}
-			/* one us delay	*/
-			msleep(10);
-		} /*max 1 seconds*/
-	} while (Retries++ < 20); /*max 20 sec*/
+	int i;
+
+	for (i = 0; i < 2000; i++) {
+		if (readl(&phbcmu->outbound_doorbell)
+				& ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
+
writel(ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE_DOORBELL_CLEAR,
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+				&phbcmu->outbound_doorbell_clear); /*clear
interrupt*/
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2584
Quoted 13 lines
drivers/scsi/arcmsr/arcmsr_hba.c +6 -0
+			return true;
+		}
+		msleep(10);
+	} /* max 20 seconds */
+
 	return false;
 }
+
 static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
 {
 	struct MessageUnit_A __iomem *reg = acb->pmuA;
@@ -2602,12 +2584,8 @@ static int arcmsr_iop_confirm(struct
 
AdapterControlBlock *acb)
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb) · line 2586
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 		if (cdb_phyaddr_hi32 != 0) {
 			struct MessageUnit_C *reg = (struct MessageUnit_C
 
*)acb->pmuC;
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
Quoted 6 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -4
-			if (cdb_phyaddr_hi32 != 0) {
-				unsigned char Retries = 0x00;
-				do {
-					printk(KERN_NOTICE "arcmsr%d:
 
cdb_phyaddr_hi32=0x%x \n", acb->adapter_index, cdb_phyaddr_hi32);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
Quoted 4 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -2
-				} while (Retries++ < 100);
-			}
+			printk(KERN_NOTICE "arcmsr%d: cdb_phyaddr_hi32=0x%x
 
\n",
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_iop_confirm(struct · line 2591
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -0
+					acb->adapter_index,
cdb_phyaddr_hi32);
Quoted 2 lines
writel(ARCMSR_SIGNATURE_SET_CONFIG,

Quoted 8 lines

&reg->msgcode_rwbuffer[0]);
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 			writel(cdb_phyaddr_hi32, &reg->msgcode_rwbuffer[1]);
 			writel(ARCMSR_INBOUND_MESG0_SET_CONFIG,
&reg->inbound_msgaddr0);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd
Quoted 1 line
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
@@ -2955,10 +2933,10 @@ static int arcmsr_bus_reset(struct scsi_cmnd

*cmd)

Quoted 64 lines

drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd *cmd)
Quoted 7 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep_again:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->outbound_msgaddr1) &
 
ARCMSR_OUTBOUND_MESG1_FIRMWARE_OK) == 0) {
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd · line 2940
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting
 
for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
drivers/scsi/arcmsr/arcmsr_hba.c static int arcmsr_bus_reset(struct scsi_cmnd · line 2942
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)
 
{
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:
waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3008
Quoted 9 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
 						return FAILED;
@@ -3025,10 +3003,10 @@ sleep_again:
 				arcmsr_hardware_reset(acb);
 				acb->acb_flags &= ~ACB_F_IOP_INITED;
 sleep:
-				arcmsr_sleep_for_bus_reset(cmd);
+				ssleep(ARCMSR_SLEEPTIME);
 				if ((readl(&reg->host_diagnostic) & 0x04) !=
 
0) {
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3010
Quoted 2 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 					printk(KERN_ERR "arcmsr%d: waiting
 
for hw bus reset return, retry=%d \n", acb->host->host_no, retry_count);
drivers/scsi/arcmsr/arcmsr_hba.c sleep_again: · line 3012
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +1 -1
-					if (retry_count > retrycount) {
+					if (retry_count > ARCMSR_RETRYCOUNT)
 
{
drivers/scsi/arcmsr/arcmsr_hba.c
Quoted 3 lines
drivers/scsi/arcmsr/arcmsr_hba.c +0 -0
 						acb->fw_flag = FW_DEADLOCK;
 						printk(KERN_ERR "arcmsr%d:
waiting for hw bus reset return, RETRY TERMINATED!! \n", acb->host->host_no);
Quoted 9 lines
return FAILED;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html