Hi Tomas, I will make time to test it. Please give me few days. -----Original Message----- From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] Sent: Friday, February 18, 2011 7:04 PM To: NickCheng Cc: linux-scsi@xxxxxxxxxxxxxxx Subject: Re: [PATCH 1/2] arcmsr: code cleanup and some corrections On 02/18/2011 02:26 AM, NickCheng wrote: > 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? > -----Original Message----- > From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] > Sent: Thursday, February 17, 2011 11:08 PM > To: 'linux-scsi@xxxxxxxxxxxxxxx' > Cc: nick.cheng@xxxxxxxxxxxx > Subject: Re: [PATCH 1/2] arcmsr: code cleanup and some corrections > > Hi Nick, > > any comments here? > > tomash > > > >> 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 <thenzl@xxxxxxxxxx> >> >> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c >> > b/drivers/scsi/arcmsr/arcmsr_hba.c > >> index 984bd52..4cd522b 100644 >> --- a/drivers/scsi/arcmsr/arcmsr_hba.c >> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c >> @@ -75,8 +75,10 @@ MODULE_AUTHOR("Nick Cheng <support@xxxxxxxxxxxx>"); >> 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(®->outbound_intstatus) & >> - ARCMSR_MU_OUTBOUND_MESSAGE0_INT) { >> - writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT, >> - ®->outbound_intstatus); >> - return true; >> - } >> - msleep(10); >> - }/*max 1 seconds*/ >> + int i; >> + >> + for (i = 0; i < 2000; i++) { >> + if (readl(®->outbound_intstatus) & >> + ARCMSR_MU_OUTBOUND_MESSAGE0_INT) { >> + writel(ARCMSR_MU_OUTBOUND_MESSAGE0_INT, >> + ®->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, >> > ®->msgcode_rwbuffer[0]); > >> writel(cdb_phyaddr_hi32, ®->msgcode_rwbuffer[1]); >> writel(ARCMSR_INBOUND_MESG0_SET_CONFIG, >> > ®->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(®->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(®->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 majordomo@xxxxxxxxxxxxxxx >> 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 majordomo@xxxxxxxxxxxxxxx > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
