>-----Original Message----- >From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] >Sent: Monday, October 17, 2016 7:27 PM >To: Sumit Saxena; linux-scsi@xxxxxxxxxxxxxxx >Cc: martin.petersen@xxxxxxxxxx; jejb@xxxxxxxxxxxxxxxxxx; Kashyap Desai >Subject: Re: [PATCH 4/7] megaraid_sas: Send SYNCHRONIZE_CACHE command to >firmware > >On 17.10.2016 15:28, Sumit Saxena wrote: >>> -----Original Message----- >>> From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] >>> Sent: Monday, October 17, 2016 6:44 PM >>> To: Sumit Saxena; linux-scsi@xxxxxxxxxxxxxxx >>> Cc: martin.petersen@xxxxxxxxxx; jejb@xxxxxxxxxxxxxxxxxx; >>> kashyap.desai@xxxxxxxxxxxx >>> Subject: Re: [PATCH 4/7] megaraid_sas: Send SYNCHRONIZE_CACHE >command >>> to firmware >>> >>> On 17.10.2016 12:24, Sumit Saxena wrote: >>>> megaraid_sas driver returns SYNCHRONIZE_CACHE command back to SCSI >>>> layer without sending it to firmware as firmware takes care of >>>> flushing >> cache. >>>> This patch will change the driver behavior wrt SYNCHRONIZE_CACHE >> handling. >>>> If underlying firmware has support to handle the SYNCHORNIZE_CACHE, >>>> driver will send it for firmware otherwise complete it back to SCSI >>>> layer with SUCCESS immediately. >>>> If Firmware handle SYNCHORNIZE_CACHE for both VD and JBOD >>>> "canHandleSyncCache" bit in scratch pad register(offset 0x00B4) will >>>> be set. >>>> >>>> This behavior can be controlled via module parameter and user can >>>> fallback to old behavior of returning SYNCHRONIZE_CACHE by driver >>>> only without sending it to firmware. >>>> >>>> Signed-off-by: Sumit Saxena <sumit.saxena@xxxxxxxxxxxx> >>>> Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxxxxxxx> >>>> --- >>>> drivers/scsi/megaraid/megaraid_sas.h | 3 +++ >>>> drivers/scsi/megaraid/megaraid_sas_base.c | 14 ++++++-------- >>>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 3 +++ >>>> drivers/scsi/megaraid/megaraid_sas_fusion.h | 2 ++ >>>> 4 files changed, 14 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/scsi/megaraid/megaraid_sas.h >>>> b/drivers/scsi/megaraid/megaraid_sas.h >>>> index ca86c88..43fd14f 100644 >>>> --- a/drivers/scsi/megaraid/megaraid_sas.h >>>> +++ b/drivers/scsi/megaraid/megaraid_sas.h >>>> @@ -1429,6 +1429,8 @@ enum FW_BOOT_CONTEXT { >>>> #define MR_MAX_REPLY_QUEUES_EXT_OFFSET_SHIFT 14 >>>> #define MR_MAX_MSIX_REG_ARRAY 16 >>>> #define MR_RDPQ_MODE_OFFSET 0X00800000 >>>> +#define MR_CAN_HANDLE_SYNC_CACHE_OFFSET 0X01000000 >>>> + >>>> /* >>>> * register set for both 1068 and 1078 controllers >>>> * structure extended for 1078 registers @@ -2140,6 +2142,7 @@ >>>> struct megasas_instance { >>>> u8 is_imr; >>>> u8 is_rdpq; >>>> bool dev_handle; >>>> + bool fw_sync_cache_support; >>>> }; >>>> struct MR_LD_VF_MAP { >>>> u32 size; >>>> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c >>>> b/drivers/scsi/megaraid/megaraid_sas_base.c >>>> index 092387f..a4a8e2f 100644 >>>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c >>>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c >>>> @@ -104,6 +104,10 @@ unsigned int scmd_timeout = >>>> MEGASAS_DEFAULT_CMD_TIMEOUT; module_param(scmd_timeout, int, >>>> S_IRUGO); MODULE_PARM_DESC(scmd_timeout, "scsi command timeout >>>> (10-90s), default 90s. See megasas_reset_timer."); >>>> >>>> +bool block_sync_cache; >>>> +module_param(block_sync_cache, bool, S_IRUGO); >>>> +MODULE_PARM_DESC(block_sync_cache, "Block SYNC CACHE by driver >>>> +Default: 0(Send it to firmware)"); >>>> + >>>> MODULE_LICENSE("GPL"); >>>> MODULE_VERSION(MEGASAS_VERSION); >>>> MODULE_AUTHOR("megaraidlinux.pdl@xxxxxxxxxxxxx"); >>>> @@ -1700,16 +1704,10 @@ megasas_queue_command(struct Scsi_Host >>> *shost, struct scsi_cmnd *scmd) >>>> goto out_done; >>>> } >>>> >>>> - switch (scmd->cmnd[0]) { >>>> - case SYNCHRONIZE_CACHE: >>>> - /* >>>> - * FW takes care of flush cache on its own >>>> - * No need to send it down >>>> - */ >>>> + if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) && >>>> + (!instance->fw_sync_cache_support)) { >>> Maybe I'm wrong, but isn't the logic inverted? >>> when fw_sync_cache_support is true it means that there is a flash >>> cache >> or a >>> battery and we don't have to send the command down ? >>> >> If "instance->fw_sync_cache_support" is set to true, it means driver >> can send SYNCHRONIZE_CACHE to firmware(firmware has infrastructure to >> support SYNCHRONIZE_CACHE). >> If "instance->fw_sync_cache_support" is set to false, it means FW does >> not support to handle SYNCHRONIZE_CACHE and FW will flush cache during >> shutdown. > >Thanks, in that case it is correct. > >>>> + if (!block_sync_cache) >>>> + instance->fw_sync_cache_support = (scratch_pad_2 & >>>> + MR_CAN_HANDLE_SYNC_CACHE_OFFSET) ? 1 : 0; > >Please add a warning or log the state of the synchronise cache command on >the >controller. Ok. thanks for pointing it out. I will add a print for the same. > > >>>> IOCInitMessage = >>>> dma_alloc_coherent(&instance->pdev->dev, >>>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.h >>>> b/drivers/scsi/megaraid/megaraid_sas_fusion.h >>>> index e3bee04..2857154 100644 >>>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.h >>>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.h >>>> @@ -72,6 +72,8 @@ >>>> #define MPI2_SUP_REPLY_POST_HOST_INDEX_OFFSET (0x0000030C) >>>> #define MPI2_REPLY_POST_HOST_INDEX_OFFSET (0x0000006C) >>>> >>>> +extern bool block_sync_cache; >>>> + >>>> /* >>>> * Raid context flags >>>> */ >> -- >> 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
