On 09/10/2014 12:15 PM, Kashyap Desai wrote: >> -----Original Message----- >> From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] >> Sent: Tuesday, September 09, 2014 7:01 PM >> To: Sumit.Saxena@xxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx >> Cc: martin.petersen@xxxxxxxxxx; hch@xxxxxxxxxxxxx; >> jbottomley@xxxxxxxxxxxxx; kashyap.desai@xxxxxxxxxxxxx; >> aradford@xxxxxxxxx >> Subject: Re: [PATCH 02/11] megaraid_sas : Use writeq for 64bit pci write > to >> avoid spinlock overhead >> >> On 09/06/2014 03:25 PM, Sumit.Saxena@xxxxxxxxxxxxx wrote: >>> Use writeq() for 64bit PCI write instead of writel() to avoid > additional lock >> overhead. >>> Signed-off-by: Sumit Saxena <sumit.saxena@xxxxxxxxxxxxx> >>> Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxxxxxxxx> >>> --- >>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> b/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> index 57b47fe..c69c1ac 100644 >>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> @@ -1065,6 +1065,13 @@ megasas_fire_cmd_fusion(struct >> megasas_instance *instance, >>> u32 req_desc_hi, >>> struct megasas_register_set __iomem *regs) >> Hi Sumit, >> the fn params are a bit confusing req_desc_lo is of type dma_addr_t and >> req_desc_hi is u32, is it possible to unite it in the future? > Agree. We should make changes here. We will do it in separate patch. > Originally fire_cmd() was written for MFI controller and carry forward for > all generation. > In MFI it use second argument as 32 bit address and third argument as > frame count, but later in Fusion adapter it started using differently. ok > >>> { >>> +#if defined(writeq) && defined(CONFIG_64BIT) >> On a similar place mpt2sas(_base_writeq) uses only "#ifndef writeq" >> if it's incorrect fix it there too or remove the CONFIG_64 here > We would like to change at mpt2sas as we have all the code with below > check for writeq() > Original discuss was started when we submitted this change in mpt2sas, but > we have delta from day-1. > LSI/Avago internal source has "#if defined(writeq) && > defined(CONFIG_64BIT)" check in mpt2sas. > > I think now writeq() is implemented in all arch, so we can safely remove > check for #if writeq(). > But we can keep this check as it is to continue for older Distribution to > take direct advantage without maintaining any separate patch. I don't know which combination of writeq and config_64bit is the right way, I was hoping that someone who knows will help with. (I'll accept almost any combination you'll post...) > >>> + u64 req_data = 0; >>> + >>> + req_data = req_desc_hi; >>> + req_data = ((req_data << 32) | (u32)req_desc_lo); >> This seems to be critical path (you are removing an spinlock to avoid >> overhead), so why do you have three consecutive assignments to the same >> variable? >> (~(u64 req_data = r_hi << 32 | r_lo)) > > Agree. We will be doing this change and re-submit the patch to address > this. Thanks. > >> Cheers, >> Tomas >> >>> + writeq(le64_to_cpu(req_data), &(regs)- >>> inbound_low_queue_port); >>> +#else >>> unsigned long flags; >>> >>> spin_lock_irqsave(&instance->hba_lock, flags); @@ -1072,6 +1079,7 >> @@ >>> megasas_fire_cmd_fusion(struct megasas_instance *instance, >>> writel(le32_to_cpu(req_desc_lo), &(regs)- >>> inbound_low_queue_port); >>> writel(le32_to_cpu(req_desc_hi), &(regs)- >>> inbound_high_queue_port); >>> spin_unlock_irqrestore(&instance->hba_lock, flags); >>> +#endif >>> } >>> >>> /** > -- > 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
